Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Today I've seen the following pattern: you have an object A and an object B. Object B accepts a pointer to A at its constructor.

Once B is created, there's a method B.doCalc() that performs a calculation (internally using A's information). The result is obtained with method B.getResult().

In order to perform another calculation, A is modified, and B.doCalc() is called again.

What is your opinion on this choice ? I would have designed it differently, but I want to hear your voice.

Edit : note that my main objection is to modify A to have a different result from B, without touching B.

Although similar, I think that just this discipline expresses a much better feeling of what's going on. Instead of

a = new A
a.whatever = 5
b = new B(a)
b.doCalc()
res = b.getResult()
a.whatever = 6
b.doCalc()
res = b.getResult()

You get the a pointer object from b itself.

a = new A
a.whatever = 5
b = new B(a)
b.doCalc()
res = b.getResult()

a = b.getAPointer()
a.whatever = 6
b.doCalc()
res = b.getResult()

because it makes more explicit the fact that a is taken from b and then modified. I still don't like it, though...

Edit 2 : one of the reasons why B accepts A at constructor is because it has to setup internal data for the calculation. These data

  • depend on the nature of A
  • they must be read from the disk, which may be slow
  • B may be run many times on the same A, albeit slightly modified A. The nature of these changes would not invalidate B's internal data. We don't want to re-load the info every time doCalc() is invoked, nor run the risk to run a doCalc(a) with B's internal data being inconsistent with the submitted A.
share|improve this question
    
It feels funny, but ... (with C# in mind) sometimes you want to create a dialog to show to the user, say when they click on an edit button. Whatever the user entered is best kept in the GUI controls themselves, for you do not want state duplication. Then you can provide accessor properties which massage the input and present it to the user. You would keep a reference to the Window in case the edit button is clicked again - you relaunch the dialog with the old data. You also might want to keep reference as a tight interface type rather than a dialog object. Cohesion&Coupling goo.gl/o641Z –  Job Feb 1 '11 at 21:39

6 Answers 6

up vote 7 down vote accepted

I realize that this problem is taken out of context, but that just feels wrong.

I see a few potential problems with it:

  1. You will be relying on the fact that B's internal implementation hasn't changed and that it still uses A in the manner you expect.
  2. Why doesn't B.doCalc() return the result in the first place, without having to call B.getResult()?
  3. Would another part of the code be potentially changing your instance of A? Would that invalidate your calculations?

Without knowing your actual constraints, it's hard to suggest another way, but I'd probably do something like this:

a = new A
a.whatever = 5 
b = new B(a) // initialize B from A, but don't keep relying on the instance of A
result = b.doCalc()

a.whatever = 7
b.setWhatever(6)
result = b.doCalc() // this calc will use the value 6

In general it seems like a case of misplaced responsibilities. If the calculation depends entirely on A, then it may be A's responsibility or B could consume A as a parameter to its doCalc() method.

share|improve this answer
    
The constraints I have are relative to B interface, which cannot be changed. The layout I depicted is actually used to produce two different results from B, by changing A and re-invoking B.doCalc(). I feel it's wrong because B changed its result, and potentially its behavior, without any visible action being performed on B itself. All the operations occurred on A. Now, this may however be reasonable in a parallel example: A is a GUI ListEntry, B is a ListWidget. You perform listEntry.setText("hello"), and then perform listWidget.sortEntries(). In this case I don't see anything wrong. –  Stefano Borini Feb 1 '11 at 21:13
    
@Stefano I feel it's wrong because B changed its result, and potentially its behavior, without any visible action being performed on B itself. << Yes! That's exactly how I feel but I just couldn't put it into words for some reason. –  Anna Lear Feb 1 '11 at 21:28
    
@Stefano GUI is a bit different, I think, because it provides visual feedback and display rules are typically not as stringent as business rules. But I agree that it is a similar example. –  Anna Lear Feb 1 '11 at 21:29
    
@Stefano If B is explicitly defined as a collection of As (or even as a wrapper around a single A), then I can see how that design would make sense. It really depends on where A and B come from. –  Anna Lear Feb 1 '11 at 21:37
    
added further comment –  Stefano Borini Feb 3 '11 at 11:18

<TL;DR>

This allows for flexibility in how the contained class operates. If you expose the details of the contained class to the client of the containing class, that becomes part of the containing class' interface.

</Tl;DR>

This is very common in UI scenarios where a widget is databound to an object. When the object changes we want the UI to display that change. Let's find another example. Let's say you have a shopping cart and want to calculate the total on the cart. One of the lineitems in the cart is updated to change the quantity. Your scenario would be

var shoppingCart =new ShoppingCart();
var lineItem=new lineItem(product,5);
shoppingCart.Add(lineItem);
var total = shoppingCart.CalculateTotal();

In the simple case CalculateTotal would look like this:

public Double CalculateTotal()
{
  var total = 0.0;
  foreach (var item in _lineItems)
  {
    total+=item.Product.Price*item.Quantity;
  }
  return total;
}

But what about the scenario where say you want to apply a discount based on quantity purchased? You COULD have the shopping cart calculate the discount or you could leave that calculation up to the lineitem. Here is how the price calculation would look in that scenario.

//LineItem.GetTotal

public double Total
{
  get
  {
    var total = Product.Price* Quantity;
    //apply a 5% discount if more than five purchased.
    if(Quantity>=5)
    {
      total-=total*.05;
    }
    return total;
  }
}

Now the calculate total function in the shopping cart is simpler.

public Double CalculateTotal()
{
  var total=0.0;
  foreach(var item in LineItems)
  {
    total+=item.Total;
  }
}

By removing the shopping cart's reliance on how the line item's total is calculated, we were able to remove the concern of calculating discounts from the shopping cart. I'm not sure if your scenario is akin to what we just discussed, but it can be extrapolated to many different cases. Going from there, the lineitem could extract it's knowledge of calculating the total to a discount strategy that could read its information from a database.

share|improve this answer
    
This principle BTW is known as the Law of Demeter en.wikipedia.org/wiki/Law_of_Demeter –  Mike Brown Feb 1 '11 at 22:53
    
added further comment –  Stefano Borini Feb 3 '11 at 11:21

Not certain why you would need to separate the docalc and getresult unless you are doing them async, but I use things along these lines when object A is a data transfer object and object B's transformation is only permitted on objects of type A for security/safety reasons sometimes.

If you do not have a good reason for the complexity is is rather awkward. I would have to know more about the example to really offer a better opinion.

EDIT:

With the added detail to the OP it looks like the original author of the code was trying to make something along he lines of a C# extension method. If you have such a facility in your language I would prefer that approach, but if you don't you might still achieve a cleaner result with a static docalc function.

share|improve this answer
    
added further comment –  Stefano Borini Feb 3 '11 at 11:19

I'm confused on why B needs to be a separate object here. If it's handling calculations with A objects, why isn't either part of the A object, or a separate function that takes an A as a parameter? Based on the information provided in the question, I would say that either of these options would be better.

share|improve this answer
    
they might be, but I have constraints to respect for consistency. –  Stefano Borini Feb 1 '11 at 20:54
    
It could be a Parameter Object - there may be quite a number of required and optional parameters that doCalc() needs. –  Michael K Feb 2 '11 at 14:02
    
added further comment –  Stefano Borini Feb 3 '11 at 11:20

I wouldn't do it that way simply because the results of doCalc() are not obvious. I'd do something more like this (borrowing from functional programming):

A a = new A();
a.setWhatever(5);

B b = new B();
res = b.doCalc(a);

This way it is obvious that doCalc() depends on object A for it's calculation.

I'm not sure why the result is being retrieved separately. Perhaps the calculation is time-intensive and the author needed the same result multiple times. The answer to that is to retain a reference to the result and pass it around rather than a reference to the object creating the result.

share|improve this answer
    
added further comment –  Stefano Borini Feb 3 '11 at 11:20

It sounds to me like the doCalc() method belongs on A. Why separate the function call out into a separate object at all? At best, object B might forward to A.

share|improve this answer
    
No, it doesn't, because B is an object encapsulating an algorithm (out of an unlimited set), and A the object encapsulating the information the algorithm needs. I can't put hundreds of algorithms on A. –  Stefano Borini Feb 2 '11 at 9:40
    
Mixins? jonaquino.blogspot.com/2005/07/… –  blueberryfields Feb 2 '11 at 15:34
    
added further comment –  Stefano Borini Feb 3 '11 at 11:19

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.