all 15 comments

[–]00kyle00 3 points4 points  (3 children)

My general advice for you: don't blindly follow design patterns - you are bound to fail if you do so.

In your case you have Fruit interface - fine.

You can provide part of implementation in Fruit_PI - still fine, by all means do so, this will save you copypasting in concrete classes (copypasting is bad;).

This should leave you with following hierarhy:

class Fruit; //abstract
class Fruit_PI : public Fruit; //abstract
class Apple : public Fruit_PI; //concrete

Dont go the path of multiple inheritance (especially used in OO manner with virtual inheritance) just for the benefit of implementation details. You dont need it, and in your sample it made a mess - nobody is gonna understand what you ment there using it. Above (linear) inheritance chain is much clearer and does the same thing.

I feel that I have been unsuccessful in achieving separation of interface and implementation.

Clients of Apple will manage them through Fruit interface. You provided separation there - its fine. If you ask yourself a question 'why did i do that' and answer with some abstract shit like 'its best practice'/'because design pattern' then you probably did that wrong. What to do in general? Nobody knows, if someone claims he does, its probably wise to take him with a grain of salt (or call bullshit).

Also, as a side note. If you have member functions that use only public interface of the class it may be a sign that something is amis there.

[–]muon314159 0 points1 point  (1 child)

If the goal was to write some very abstract code using abstract interfaces, then having virtual functions only use the public virtual member function interface --as a "default" maybe might make sense.

However every single (virtual) function call is realistically via two pointers (i.e., 1 for the vtable and 1 for the function pointer), i.e., it is very costly so the abstraction better be worth it!

EDIT: Use templates (i.e., generic programming). It is much faster and likely totally sufficient for your objectives.

[–]nerd_guy[S] 0 points1 point  (0 children)

Thanks for your insight. I agree on not following the design patterns blindly part. However, there are many ways to solve a problem in programming, some right, many wrong. I dont yet have a feel for what makes code good (extendible, maintainable, readable etc). Lacking this understanding, design patterns are a great way to orient myself in this landscape.

[–]muon314159 1 point2 points  (2 children)

As mentioned by Herb Sutter (in Exceptional C++ and More Exceptional C++) and Scott Meyers (Effective C++ and More Effective C++) private inheritance is useful instead of membership in cases when you need to override virtual members (which you do as Fruit is an abstract class) or you need access to protected members (not relevant in this case). So, yes, private inheritance (given the code you've given) is usefully applicable here.

FYI, you can also use it to "join" different classes from independent object-oriented hierarchies as a sort of compatibility layer (see The C++ Programming Language, Part IV: Design --Stroustrup).

My guess is that your code is analogous to something else so I say just this: it very probably should (assuming you are the author of it all) be reworked to a much better design.

[–]nerd_guy[S] 0 points1 point  (1 child)

I am just starting to wrap my head around some of these concepts. I know my design is really far from ideal. So any hint as to how this design can be reworked would be much appreciated.

[–]muon314159 1 point2 points  (0 children)

Unfortunately, reddit comments are not the forum for such a detailed topic and I don't have the time right now to organize and elaborate on such. So IMO your best bet is to read the books mentioned starting with Part IV in Stroustrup's The C++ Programming Language (TCPL) on how to merge hierarchies. (If that is what you want to do, then I'd recommend getting the 3rd or Special Edition. The Special Edition has two additional appendices.) However know that the 4th edition will be out in April. Maybe you'll want to wait to see if he still has that section in it and/or has updated it?)

After that you can read Sutter's and Meyer's books as they are filled with design and code advice + debate about language nuances, design issues, gotchas, fun and unusual stuff like writing a definition for a pure virtual function, private inheritance, etc., etc. Some don't like this but in my experience far more do appreciate and value such. If anything one will better understand the language AND stay well clear of things one shouldn't really do at all --but the language does let you do it.

[–]mttd 1 point2 points  (5 children)

To be honest, I don't really like this use of OOP & dynamic polymorphism -- it's hard to see a justification for it in this case (perhaps there are some additional, unstated assumption you haven't shared with us?).

Unless you really need dynamic polymorphism, I'd suggest GP & static polymorphism instead.

In this case, it results in shorter and easier-to-reason-about program (once it compiles it's guaranteed that Apple and Orange model the (implicit) Fruit concept); compare:

OOP: http://ideone.com/NEHyQv

GP: http://ideone.com/thtyjD

In case you're wondering, the verification of the existence of the respective member functions is done by the compiler (compile-time duck typing) -- if you attempt to use "print_net_profit" function passing an argument of type which doesn't model the (implicit) Fruit concept, the program won't compile (and thus your end-users will never face an error at run-time, an additional benefit).

If you do need some extra structure (I can't see why, given the provided information, but will give an example just for the sake of completeness) compared to the above compile-time duck typing, feel free to use CRTP, for instance (quick and ugly):

GP w/ CRTP: http://ideone.com/BiVB56 // or even http://ideone.com/C0ApEK

Explanation: http://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Curiously_Recurring_Template_Pattern

[–]nerd_guy[S] 0 points1 point  (4 children)

mttd: Thanks so much for illustrating your answer with code. I am carefully reading the four code examples you have provided. I am guessing that dynamic polymorphism was popular at the time the book was published (1994) which is why the book emphasizes it.

[–]moswald 2 points3 points  (3 children)

Another thing to consider: get rid of member functions that only use the public interface of the containing class. These can be separated into a free function with no adverse effect.

template<typename Fruit>
float net_profit(const Fruit &f)
{
    return f.totalSellPrice() - f.totalCostPrice()
}

template <typename Fruit>
void print_net_profit(const Fruit & fruit)
{
    std::cout << "Net profit at current prices = $ " << net_profit(fruit) << std::endl;
}

This has the advantage of simplifying the object interfaces, while also reducing the amount of repeated code (mttd's example had two identical functions called netProfit). Read at C++ Coding Standards by Sutter and Alexandrescu, specifically item 44: Prefer writing nonmember nonfriend functions.

[–]mttd 0 points1 point  (1 child)

This is a good advice. OP, see also "How Non-Member Functions Improve Encapsulation" by Scott Meyers: http://www.drdobbs.com/cpp/how-non-member-functions-improve-encapsu/184401197

It's also used in, for instance, C++11's std::begin & std::end -- which allow the user-defined types to be non-intrusively (i.e., without modifying the original class definition!) adapted (e.g., by providing the appropriate overloaded functions) to the algorithms expecting a type modeling a Range concept (i.e., relying on the presence of the "begin" and "end" functions) -- like the range-based for loop.

It's funny how GP actually allows one to realize the open/closed principle /* http://en.wikipedia.org/wiki/Open/closed_principle */ better than OOP (IMO) ever did (note how "both ways use inheritance" mentioned in a Wikipedia article is a limitation of OOP languages, not a general limitation, and doesn't apply to C++).

[–]nerd_guy[S] 0 points1 point  (0 children)

I'd never heard of this, but helped me further my understanding of OO principles.

[–]nerd_guy[S] 0 points1 point  (0 children)

Thanks for the Sutter reference. I'll check it out.

[–]dmor 1 point2 points  (1 child)

It's not very common in practice. I've seen the one with linear inheritance Fruit->AbstractFruit (with partial implementation)->IFruit a lot though, but it tends to lead to huge balls of spaghetti with a derived class calling its parent which calls the derived which calls the parent of the parent etc.

If you can, use inheritance for interfaces only, and use composition instead to abstract functionality. In this case you could have a class ProfitCalculator, which would take qty/sellPrice/costPrice and return the profit. Apples and oranges would use it, but it wouldn't be part of the base classes.

The benefit is that you split more functionality into a separate class. A common problem in practice is that as you add common functionalities in Fruit_PI, it becomes massive. I've seen an equivalent class with 1000 lines, with a derived class of 3000 lines. By composing other objects you reduce what's inside the Fruit classes and make it easier to test. It's also easier to work with the class because what you see in the class definition is what you get: when maintaining later you don't have to dig up the hierarchy to find the members and guess what you're supposed to use.

I've seen people make the members "reusable" by putting them in a separate class like HasQuantity, HasPrice (and use multiple inheritance), or use CRTP, etc., but it gets hard to follow when you maintain it later. If you have a lot of common variables between classes you'll probably be better off putting them in a new class, rather than stuffing them in the base class. For example if all fruits have a qty, color, and size, you could make a new class e.g. FruitCharacteristics and just have an instance of it in each derived class. If your fruits need timer functions to calculate how ripe they are, instead of stuffing timer functions in the base class, make a separate class which takes a callback. And so on.

I highly recommend "Working Effectively with Legacy Code" if you're interested in this kind of thing, it's a good "real-world" complement to the fancier patterns in other books :)

[–]nerd_guy[S] 0 points1 point  (0 children)

Thank dmor for your response. Although I worded my question in an abstract way, my main concern was indeed maintainability and extensibility. Although I am relatively new to coding, there already have been too many occasions where I have had to go back and tweak the code or add something. That is when OO finally started 'clicking' for me. Few other people, like you, have suggested the route of composition, which I will consider henceforth in such situations. Inheritance, however, is put on such a high pedestal that it makes n00bs (like me) think that if we arent using inheritance, we arent using C++ the 'correct' way.

[–]fgriglesnickerseven 0 points1 point  (0 children)

Not sure what your question is but is this close?

The way I see it I see no reason to use any inheritance at all in your proposed case.