An unwise idiom?

Started by
41 comments, last by MaulingMonkey 17 years, 11 months ago
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?
Advertisement
Why not just have a "spouse" member?
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 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
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.
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.
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)
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.
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.
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?

This topic is closed to new replies.

Advertisement