• Advertisement
Sign in to follow this  

An unwise idiom?

This topic is 4308 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

So there are several places in my code where two objects reference each other (for the sake of argument, include 1:1 cardinality). So a Man object might have a m_husband reference to a Woman object which then has a m_wife reference back to him. The references should be mutually consistent, of course;
man->m_wife->m_husband == man
should always be true, for instance. Now, the tricky part is where the invariant is temporarily violated in a SetHusband or SetWife method. Here's a few approaches I've used in the past, of varying wisdom. The "shit, infinite loop" approach
void Man::SetWife(Woman* wife)
{
    m_wife = wife;
    m_wife->SetHusband(this);
}

void Woman::SetHusband(Man* husband)
{
    m_husband = husband;    m_husband->SetWife(this);
}
The "whatever you do, don't call THAT one" approach
void Man::SetWife(Woman* wife)
{
    m_wife = wife;
    m_wife->SetHusband(this);
}

void Woman::SetHusband(Man* husband) // DON'T CALL THIS METHOD EXCEPT FROM Man::SetWife!!!!!!
{
    assert(husband->GetWife() == this);
    m_husband = husband;
}
The "be nice to each other's internals" approach
// Man and Woman have each other as friend classes
void Man::SetWife(Woman* wife)
{
    m_wife = wife;
    m_wife->m_husband = this;
}

void Woman::SetHusband(Man* husband)
{
    m_husband = husband;
    m_husband->m_wife = this;
}
The "I now pronounce you man and wife" approach
void Marry(Man* husband, Woman* wife) // Friend function of Man and Woman
{
    husband->m_wife = wife;
    wife->m_husband = husband;
}
The "I heard you the first time" approach
void Man::SetWife(Woman* wife)
{
    if(m_wife != wife)
    {
        m_wife = wife;
        m_wife->SetHusband(this);
    }
}

void Woman::SetHusband(Man* husband)
{
    if(m_husband != husband)
    {
        m_husband = husband;
        m_husband->SetWife(this);
    }
}
The first approach obviously doesn't work. The second is what I have foolishly changed the first to, in times where I don't have the willpower to do anything less hackish. I don't like the third one because of the way it breaks encapsulation and makes coupling stronger. The fourth one is cute, but I dislike splitting out the functionality like that and increasing code repetition. So I've been doing the fifth one more and more. The big thing I don't like about the fifth method, though, is that it is transparently idempotent. Setting the wife to something it's already set to, in my opinion, should raise warning flags, and there is no way I can think of to do that. I've expressed this all in terms of mutual references, BTW, but the general case I'm interested in is two classes which in any way have a mutual state dependence and where a change to part of the dependent state might be initiated from either side. What do y'all think? What have you used in the past? Is there a Sixth Way I'm not considering?

Share this post


Link to post
Share on other sites
Advertisement
Guest Anonymous Poster
Why not just have a "spouse" member?

Share this post


Link to post
Share on other sites
Quote:
Original post by Anonymous Poster
Why not just have a "spouse" member?

That doesn't really have anything to do with the issue. FWIW, Man and Woman are not related via inheritance.

Share this post


Link to post
Share on other sites
I am all for the spouse approach...

make a base class of a person that holds a spouse..
Have a function in the base class that addsSpouse(person * spouse)
derive the Husband and Wife from the person class.

You can then add spouses at will. This also allows for same sex marrages just in case. :)

theTroll

Share this post


Link to post
Share on other sites
Er, I don't think you're quite understanding the issue at hand. It really has nothing to do with "husband" and "wife" being differently named. It's just about maintaining consistent dependent state. The problem would remain unchanged if the changes you propose were made.

Perhaps it's a matter of the example I used.. If you like, instead of "husband" and "wife", think of it as "GuyWhoWearsAHat" with a member m_hat, and a "HatBeingWornByAGuy" class with a member m_guyWearingMe.

Share this post


Link to post
Share on other sites
Creating a Spouse class doesn't solve the problem, anyway; you'll still have the problem of updating the two separately.

The only thing I can think of off the top of my head is to have a 'Marriage' class or equivalent - I'm not sure how practical this idea would be in your actual problem - which contains details of the involved parties. I'm sure I don't need to describe how this would work. I like it because it's nicely encapsulated, but I also dislike spreading out functionality like this. Could be worth investigating though.

Share this post


Link to post
Share on other sites
I don't see what's wrong with method #4 (using a "marry" function)

Basically, you have one goal - to "marry" a man and woman, and you set the additional restriction that it be "atomic" in some sense. So it's problematic to try and break it up into two different operations, since the true goal is to have it as one operation anyway.

Another key thing is that the "marry" operation doesn't really belong to the man or the woman - unless you have some kind of "minister" or "judge" class that can have a marry(Man*, Woman*) function, the marry() function doesn't have to be in a class (since it has some sort of "anonymous" actor).

EDIT: addressing the "guy wearing hat" scenario, while a wear(Man*, Hat*) may work in the same case, it may be better to consider that as Man::wear(Hat*) in the vein of solution #2 (since hats do not typically wear men)

Share this post


Link to post
Share on other sites
I've used something like this before (if my memory serves me correct [smile])


void Man::SetWife(Woman* woman)
{
// check if same
if (m_wife == woman)
return;

// divorce
Woman* oldWife = m_wife;
// set m_wife to null, so when oldWife calls SetWife(0) on this, we'll skip out early
m_wife = 0;
if (oldWife)
oldWife->SetHusband(0);

// marry
m_wife = woman;
if (m_wife)
m_wife->SetHusband(this);
}

void Woman::SetHusband(Man* man)
{
// check if same
if (m_husband == man)
return;

// divorce
Man* oldHusband = m_husband;
m_husband = 0;
if (oldHusband)
oldHusband->SetWife(0);

// marry
m_husband = man;
if (m_husband)
m_husband->SetWife(this);
}




-- EDIT --

LoL, there was a lot of replies between me clicking reply, and writing this up :P

I think what I focus on in mine is to keep the coupling always only between 2 objects, so to divorce your old coupling before creating the new one, and without accessing any private members on the other object. You could obviously make this simpler, without the reentrancy "hacks", by directly accessing members of the other object.

Share this post


Link to post
Share on other sites
Quote:
Original post by etothex
I don't see what's wrong with method #4 (using a "marry" function)

The problem is that the change in state may actually involve a large amount of internal activity on the part of both Man and Woman. Because that activity is specific to each class, putting it in a Marry method increases coupling between the classes. Even if Man and Woman have separate maintainers, both have to maintain the Marry function.

Share this post


Link to post
Share on other sites
Quote:
Original post by c2_0
I've used something like this before (if my memory serves me correct [smile])

*** Source Snippet Removed ***

Looks like method five, with additional thought paid to "remarriage" (which is a very good point....definitely something that should be a part of any approach). What are your thoughts on the idempotence issue? In fact, the way you've presented it makes it even more pressing: Should remarrying the same woman involve divorcing her first?

Share this post


Link to post
Share on other sites
Quote:
Original post by Sneftel
Quote:
Original post by etothex
I don't see what's wrong with method #4 (using a "marry" function)

The problem is that the change in state may actually involve a large amount of internal activity on the part of both Man and Woman. Because that activity is specific to each class, putting it in a Marry method increases coupling between the classes. Even if Man and Woman have separate maintainers, both have to maintain the Marry function.


Hmmm... I suppose it depends on how more "complicated" you would make the system. For example, consider:


// SetWife, SetHusband should be private
void Man::SetWife(Woman* wife)
{
m_wife = wife;
}

void Woman::SetHusband(Man* husband)
{
m_husband = husband;
}

void Marry(Man* husband, Woman* wife) // Friend function of Man and Woman
{
husband->SetWife(wife);
wife->SetHusband(husband);
}



Reasoning being, there is no reason why the wife's function "SetHusband" should have to have anything to do with changing the state of the husband. SetHusband does exactly what it is needed to do - set the wife's husband - and that can be described in the pre/post conditions of those functions (which, since they are private, only need to be known to the maintainer of that class and the marry function)

Share this post


Link to post
Share on other sites
Quote:
Original post by visage
I dunno. Tough issue. Pretty much the same as what you already have.

Actually, not quite.... it looks like you've solved the idempotence issue.

Share this post


Link to post
Share on other sites
Quote:
Original post by Sneftel
Looks like method five, with additional thought paid to "remarriage" (which is a very good point....definitely something that should be a part of any approach). What are your thoughts on the idempotence issue? In fact, the way you've presented it makes it even more pressing: Should remarrying the same woman involve divorcing her first?


I must admit I didn't give that too much thought, I tried to rember how I solved a similar issue before. Right now I'm wondering if it might be possible to come up with a more general Coupling class, or mechanism, which more gracefully handles these problems...

Share this post


Link to post
Share on other sites
Quote:
Original post by etothex
For example, consider:

*** Source Snippet Removed ***

Good point. The internal functions could then handle all the dirty work, without requiring complexity in Marry(). The point about SetHusband/SetWife being private is well taken... the reason I accessed the class members directly in my example is that a bare SetHusband/SetWife would violate invariants, but private functions don't need to worry about that.

Share this post


Link to post
Share on other sites
I'd likely go with the "I heard you the first time" approach, and/or having a separate Marry function. If I was thinking in terms of abstract design aesthetics, I would prefer the Marry() function, since it seems neater and more balanced. If I was thinking about it from the perspective of being a practical programmer who just wants to get on with it in a reasonably safe way, I would probably go with "I heard you the first time" (although in fact it probably doesn't have any practical advantages over a separate Marry function).

If you wanted to be excessively OO about it, you could create a Marriage class to encapsulate the relationship itself*, but I doubt it's worth it, usually, and of course you then have to decide how to design the Marriage class and how to interface it with the Man and Woman classes.
Actually, given that you're talking about state dependencies, then perhaps it would be possible to encapsulate the part of the state that is dependent on both objects.

I also found it quite funny that you were talking about "making coupling stronger" being a problem, given your example.

Edit: Oh, I see hymerman already suggested a Marriage class... as did c2_0

Any chance of a more real-world example?

John B

* I'm thinking something along the lines of: Man and Woman could each have a SetMarriage(), and the Marriage object holds references to the Man and Woman involved; so you create a Marriage() object and then SetMarriage() on each person. Marriage should perhaps be called MarriageCertificate.

Share this post


Link to post
Share on other sites
Quote:
Original post by JohnBSmall
* I'm thinking something along the lines of: Man and Woman could each have a SetMarriage(), and the Marriage object holds references to the Man and Woman involved; so you create a Marriage() object and then SetMarriage() on each person. Marriage should perhaps be called MarriageCertificate.

That is definitely a Sixth Way. Probably not appropriate for the hat example, but in situations where the relation has "a life of its own" that may well be an option to consider.

Quote:
I also found it quite funny that you were talking about "making coupling stronger" being a problem, given your example.

Yes... the irony of designing a husband and wife to have as little to do with each other as possible was not lost on me. [grin]

Share this post


Link to post
Share on other sites
I like the third approach, myself. As far as breaking encapsulation goes, OO and encapsulation are there to make coding easier to write/read/maintain. When the code becomes more complex just to avoid "breaking encapsulation", it kind of defeats the purpose.

Share this post


Link to post
Share on other sites
Quote:
Original post by JohnBSmall
* I'm thinking something along the lines of: Man and Woman could each have a SetMarriage(), and the Marriage object holds references to the Man and Woman involved; so you create a Marriage() object and then SetMarriage() on each person. Marriage should perhaps be called MarriageCertificate.


The problem I'd see with that is that you could potentially create a Marriage object with no people in it. I was going to suggest having it so that you construct a Marriage object with a Man and a Woman object, and the constructor sets both objects to point to itself, but then I realised that that's really just a rephrasing of method 4 (in the constructor of a friend class, instead of a single friend function).

I think my favourite approach seen here so far is ethotex's one... by making the private function on each object responsible for only updating itself, and then providing a public free function that forces you to call both private functions, I'd say you're actually being /more/ efficient with your code (as all the object-pair wranging is being done in a single shared function, instead of in both Man and Woman). If you don't like the free function aspect of it, you can always add a call-through member to each class (i.e. man->Marry(woman) == Marry(man, woman), and woman->Marry(man) == Marry(man, woman)).

Share this post


Link to post
Share on other sites
For some reason, the idea of a "marriage" object just seems like a nasty solution to me. I think that would be going over the top on the OO. :)

Share this post


Link to post
Share on other sites
Quote:
Original post by etothex
For some reason, the idea of a "marriage" object just seems like a nasty solution to me. I think that would be going over the top on the OO. :)

Depends on the situation, I think. For this, I agree.

Of course, if you really wanted to go over the top, you would create a Priest class to perform the ceremony.

John B

Share this post


Link to post
Share on other sites
Quote:
Original post by etothex
For some reason, the idea of a "marriage" object just seems like a nasty solution to me. I think that would be going over the top on the OO. :)


I agree completely. If it were me, I'd just use the I heard you the first time method and be done with it.

Share this post


Link to post
Share on other sites
Why, exactly, do the husband and wife have to know about each other?
Haven't used C++ in a while, so python'll have to do:

class Person:
...

husband = Person()
wife = Person()
couple = [husband, wife]


Somehow I feel this circumvents your main point, which means you should probably find a better example :)

Share this post


Link to post
Share on other sites
Maybe for this single example, it's more practical to not go overboard with OO, and "be done with it"...

However, coupling, be it one-to-one or many-to-one, is a pretty common pattern, and usually involves having make sure the coupling is always mutual and atomic, the coupling is broken when one object is destroyed, etc... so, I'd say it's worthy of an OO reusable design. If implemented neatly, it should be a case of write once, use forever.

Share this post


Link to post
Share on other sites
I agree this is a rather simple situation. For example, consider the fact that we may want in the future to add a "dating" relationship/coupling, or an "engaged" coupling, etc. However, I'm sure there are better ways to do those than coming up with a "relationship" class and deriving "marriage", "dating", etc.

Unfortunately, the "write-once/reuse" thing is only ideal. In this case, the best solution would be to design it as simply as possible(for marriage case) and then kind of "refactor" it (even though it's not traditional refactoring) to make other situations work.

Share this post


Link to post
Share on other sites
Quote:
Original post by Sneftel
Quote:
Original post by Anonymous Poster
Why not just have a "spouse" member?

That doesn't really have anything to do with the issue. FWIW, Man and Woman are not related via inheritance.


I should hope not if they're getting married! ;)

Although that might be exactly what's needed - or else make the classes friends of each other. That allows the same kind of approach as the free Marry(), except initiat-able from either side:


void Man::marry(Woman* w) {
if (m_wife) { m_wife->m_husband = 0; }
m_wife = w;
m_wife->m_husband = this;
}
// and vice-versa.


That's still "transparently idempotent", but you could easily add a check (and pretend it's for "optimization" if you have to ;) ) to make the idempotence more explicit (or for that matter, raise warning flags if you feel you ought to). It also avoids the feeling of mutual recursion (properly guarded or not). On the other hand, it's trading having one function that's coupled to two classes, for the two classes being coupled to each other in a way that they weren't quite before... (well, I guess you could have public marry() and also private/protected setSpouse() which is a helper? :s)

Then again, if relationships should be "till destruction do us part", then maybe you should either make a wrapper object, or have some sort of ... er... CoupleFactory? o_O

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement