views:

391

answers:

1

Looking on this answer I understand that you should not copy private pointers using friendship in C++ like I did in my program:

class bar;

class foo
{
private:
   some_smart_pointer<int> state;

public:
   foo(int value) : state(new int(value)) {}
   friend class baz;
};

class bar
{
private:
   some_weak_pointer<int> state;
public:
   friend class baz;
};

class baz
{
public:
   foo Foo;
   bar Bar;

   baz() { Bar.state = Foo.state; }
};

That's ok but passing a constant smart pointer violates the principle of encapsulation.
It exposes the internals of the system. Passing bar to foo also creates coupling. Also in my system it makes sense to aggregate objects to what is called in that question a fluent interface:

MainObject.PropertyCollection.Property

As it's said in that SO answer it does not violate the Law of Demeter or does it?
I'm extremely confused here.
I'm redesigning my library for the last time and I want it to be right.
I thought that instead of passing my internal smart pointer I should pass a wrapper class that will also be used as a property and set it to read only and then copying is safe right?

EDIT according to comments: Well I'm implementing the D20 3.5 SRD (http://www.d20srd.org/).
For instance there are six abilities.
In my design D20Abilities is aggregated inside D20Character.
D20Abilities contains 6 D20Ability instances: Strength, Dexterity, Constitution. Wisdom, Intelligence and Charisma.
All of the D20Ability instances hold a private state (which is called modifier) that is shared throughout the system. For instance each skill needs his own ability's modifier.

+2  A: 

In regards to your description and the Law of Demeter. Using the example from wikipedia, it is said to prefer:

void exercise(const Dog& d) { return d.walk(); }

instead of:

void exercise(const Dog& d) { return d.legs.move(); }

Because the dog should know how to walk instead of having its legs moved around. Applying this to your D&D program, we'd get something like, instead of:

bool hit_orc_with_long_sword(const Character& c, int DCValue) {
   return c.D20Abilities.Strength() + d20.roll() > DCValue;
}

Prefer: skill_check(c, MeleeAttack(LongSword(), Orc());

bool skill_check(const Character& c, const Action& a) {
    return c.attempt(a);
}

//.... in class character:

bool Character::attempt(const Action& a)
{
    return a.check_against(D20Abilities);
}

//.... in class Action:

bool Action::check_against(const D20Abilities& a) {
   return GetRelevantStat(a) + d20.roll() > DCValue;
}

This way, instead of accessing through property groups, Characters know how to perform Actions, and Actions know how likely they are to succeed with certain stats. It decouples the interface. Consumer code (like the hit_orc_with_long_sword above), doesn't have to "tell" the Character how to check his strength, it's handled by the interaction of classes, each handling a well defined domain.

However, the code sample provided does look like it has several problems. When you are using friend to do anything but implement certain operators, I find it points to a design problem. You shouldn't be throwing around internal state from one object to another; objects should know how to perform a set of actions from their own internal state and the arguments passed to them as functions.

Todd Gardner
quant_dev
Todd Gardner
I am sharing internal state because the calculating a skill for example depeands on a certain ability. For instance the knowledge skill:Total = Rank + Intelligence modifier + MiscI get the intelligence modifier by passing a pointer from a D20Ability instance to D20Skill instance.
the_drow
could you please comment?
the_drow