views:

41

answers:

3

Hello,

A class A possesses an instance c of a class C. Another class B has to modify c through C::setBlah(); method.

Is it bad to create an accessor C getC(); in A and then use A.getC().setBlah() ?

Or should I create a method A::setBlah(); that would call C::setBlah(); ? Isn't it annoying if there are several methods like that ?

+1  A: 

If C has many similar methods to be called by classes outside A, I would definitely not add these to A. I prefer to keep interfaces as minimal as possible. If these changes are related and are done together, you may add a dedicated single function to A to execute all calls to C at once and logically collect all these changes under an intuitive name.

Such a setup also raises the question: why does B need to touch A's member C? Maybe your design is not quite right - should C be a member of B rather than A?

Péter Török
+1  A: 

A Method C getC(); creates a copy of c, so calling A.getC().setBlah() would modify the copy of c, not the c of A.

struppi
doublep
Yes, of course, thank you :)
Klaus
+2  A: 

As with most "is it bad to do X?" questions, the answer is that it depends entirely on a given situation.

Sometimes it might be a really bad idea to have a getC() sort of function because it breaks encapsulation. Other times it might be completely fine because encapsulation of that detail might be irrelevant, and writing a lot of wrapper functions increases the amount of code that you have to write.

Pick whichever makes the most sense for the given situation. In code that I've written, I've taken both approaches.

If you do go the getC() route, do make sure you return a reference; otherwise you'll be modifying a copy which doesn't sound like what you want. Or, you might consider making A::c public so that you don't need a function at all.

A third option to consider would be inheritance from C, which removes the need for getC() or wrapper functions in A.

James McNellis
I'd say that the decision should mostly depend on whether `C` is an internal detail type (use `A::setBlah()`), or some public interface (use `a.getC().setBlah()`). Of course this is a somewhat fuzzy concept if you are not writing a library as often it is not a problem to make internal detail a public interface or vice versa in this case.
doublep
Thank you! I'll think about that :)
Klaus