Parent/child relationship

[Don Kiddick]

I have a parent (Portfolio)/child (Trade) relationship, where each portfolio has 0 or more trades, and each trade has exactly on portfolio.

I currently have this coded as follows :

 
class Portfolio {
 
  List<Trade> trades;
 
  void addTrade(Trade trade) {
    trades.add(trade);
    trade.setParent(this);
  }
}
 
class Trade {
 
  Portfolio parent;
 
  void setParent(Portfolio portfolio) {
    this.portfolio = portfolio;
  }
 
}

However this doesn't capture the requirement that Trades always have exactly one Portfolio, ie, parent cannot be null. I would ideally like the code to reflect this knowledge.

The other way I thought to do it is :

  
 
class Portfolio {
 
  List<Trade> trades;
 
  void addTrade(Trade trade) {
    trades.add(trade);
  }
}
 
class Trade {
 
  Portfolio parent;
 
  Trade(Portfolio parent) {
    setParent(parent);
  }
 

  void setParent(Portfolio portfolio) {
    checkPortfolio();
    this.portfolio = portfolio;
    portfolio.addTrade(this);
  }
 
  void checkPortfolio(Portfolio portfolio) {
    if (portfolio == null) {
      throw new IlleagalArgumentException();
    }
  }
 
}
 

I worry that this makes my code less testable as it is more hassle to create trades now.
Also during the trade constructor, portfolio.addTrade() gets called, which isn't ideal (calling parameters methods within a constructor.

After writing this down, I'm beginning to think that the 2nd style is the best.
Does anyone have any views on this and/or patterns that they use to handle this situation?

thanks, Don.

[Ilja Preuss]

Here is another idea:

class Portfolio {
 
  ...
 
  Trade addTrade() {
    Trade trade = new Trade();
    trade.setParent(this);
    trades.add(trade);
    return trade;
  }
}

Makes adding a trade a breeze *and* ensures integrity.

As an aside, you might want to use an assert statement instead of your checkPortfolio method:

  void setParent(Portfolio portfolio) {
    assert portfolio != null : "parent portfolio required";
    this.parent = portfolio;
  }

You need to explicitly enable assertions at runtime to let this have an effect, which might be both an advantage and a disadvantage.

[Don Kiddick]

thanks for the reply.

With that example you can still call "new Trade()" independently of a portfolio though, leaving the parent null.

I'm beginning to think option 2 (trade constructor takes a portfolio) is my preffered one.

What is the advantage of using an assert over IllegalArguementException? I think I always want my assertions on.

thanks, Don.

[Ilja Preuss]

Originally posted by Don Kiddick:
With that example you can still call "new Trade()" independently of a portfolio though, leaving the parent null.

I'm beginning to think option 2 (trade constructor takes a portfolio) is my preffered one.

Right. What I actually wanted to write was

Trade addTrade() {
    Trade trade = new Trade(this);
    trades.add(trade);
    return trade;
}

What is the advantage of using an assert over IllegalArguementException? I think I always want my assertions on.

Mostly less code, as far as I can tell...

[Don Kiddick]

Ilja, thanks for your time.
Don.

[Don Kiddick]

One thing I've noticed using the assert keyword is that it's really hard to test using junit that they are thrown.

e.g.

 
@Test  
public void testExceptionThrownWhanNameIsInvalid() {
  try {
    new Product("My invalid name<:@!@:");
    fail();
  catch (AssertionError e) {
    //Success
  }
}

I don't know if this is a junit 4.0 thing, but fail() now throws a AssertionError, thus making this test always pass!

[Ilja Preuss]

Never occurred to me... I typically don't think of asserts as expected behavior (just like logging), so I don't write tests for them.

But I guess you could also rewrite the test as

 
@Test(expected=AssertionError.class)
public void testExceptionThrownWhanNameIsInvalid() {
  new Product("My invalid name<:@!@:");
}

As an aside, since JUnit 4 I prefer my test method names to read more like specification of behaviour (BDD-like), for example "throwsExceptionWhenNameIsInvalid".

[Don Kiddick]

yeah was wondering about that, seeing as we don't have to prepend test to our test methods anymore.