CoreTheApples Problem: You have a bag of Fruit. You wish to core the Apples. '''please add your solution''' Possible Solutions: * Go through each item in the bag, if it is an Apple, you core it. * foreach { if isa::Apple core(apple) } * while(fruits.hasNext()){ fruit = (fruit)fruits.next(); if (fruit instanceof Apple) ((Apple)fruit).core(); } * Create a function from fruit types to preparations; for each fruit in the bag, invoke the method returned by this function on the fruit. In this particular case, the function would return Do''''''Nothing() for any fruit that wasn't an apple and Core() for the apples. * Add a Core function to Fruit it is a No-Op. Over-Ride in Apple to perform the function. * (Similar to previous) Add a no-op prepare() method to Fruit, In Apple, define prepare() as core(). You can then modify prepare() to do other things besides core it. Other fruits define prepare as appropriate. You can supply a Preparer object to a Fruit that knows a bit more about different ways of preparing fruit depending on the target recipe. Banana.prepare() { peel(); slice(); }, if you are making a banana split. Banana.prepare() { peel(); mash(); }if you are making pie. ''CodeSmell. I would accept that the pie might know how it should be made, or that a recipe or a cook knows how to combine ingredients. But add_self_to(Recipe pie) seeems to be asking for trouble. What if order matters? What if the banana adds itself before the pastry? What if you want to layer meat and lasagna sheets?'' So have the Fruit implement a prepareWith(Preparer tool) that allows a Recipe to specify which tool is used to prepare the Fruit (and the order, potentially). The point is not the exact form of the operation, just that "core the apples" is potentially too specific for the task of cooking, and what you really mean is "prepare the ingredient", as in "dice the onions, press the garlic, peel the apples, sift the flour, whip the cream, saute the mushrooms, cut the cheese, etc. etc. What about: * Ask the bag for the apples. Core each. ---- ItDepends. Why are you coring the apples? If you meet a banana, will it be peeled? Before we can answer your question we need to decide which object knows that apples need to be cored. The apple? Probably not. Maybe the recipe? If so, let the apple know how to core itself and let the recipe invoke that as needed. new Fruit''''Salad(Bag fruits) { foreach fruit in ( fruits ) { fruit.wash(); fruit.peel(); fruit.pip(); self.add(fruit); } } Perhaps the real question is who has responsibility for selecting ingredients - the recipe, a cook, the ingredients themselves or ...? If we say the cook then... new Apple''''Pie(Pastry, Apples apples, Sugar ... ) { throw error if apples.weight() < X ... foreach ( apple ) { apple.core(); apple.slice(); // Problem here - is a sliced apple still an apple? ... } } If we say the recipe has responsibility for selecting ingredients then... new Apple''''Pie(Larder larder, Fridge fridge, ...) { new apples = grep /apple/ (larder->fruitbag); // guess MyFavouriteLanguage :-) throw error if apples < X; } Hmm. There's a CodeSmell there. A recipe shouldn't know that much about your kitchen. ''Perhaps a Kitchen is a framework for executing Recipes?'' Perhaps each recipe has a list of ingredients? ExerciseForTheReader. In summary, ItDepends. DoSimpleThings, and add indirection when you actually need it and no sooner. ''I think that the first proposed solution is the best. It does what is needed and no more. There is nothing in the problem description to indicate the need for anything other than finding all '''Apple'''s and calling '''core()''' on them. What is the importance of the '''Fruit''' class if all we care about are '''Apple'''s? Why not just iterate through a list of '''Object'''s?'' public void coreAllApples(List bagOfFruit) { Iterator fruitIterator = bagOfFruit.iterator(); while ( fruitIterator.hasNext() ) { Object fruit = fruitIterator.next(); if (fruit instanceof Apple) { Apple apple = (Apple) fruit; apple.core(); } } } ---- OK, now peel all the bananas, pit all the grapes, then prepare, in a way appropriate to the task at hand that is analogous to coring applies, avocados, mangos, lychee, pineapples, guava, papaya, carambola, cherimoya, figs, gooseberries, and jujubes. And yes, these are all real fruits. ---- But in coring apples, pitting grapes, peeling bananas and hulling strawberries, all you are doing is removing the non-edible parts of the fruit, so perhaps you then should try: while(fruitbag.hasNext()) { Fruit fruit = fruitbag.next(); fruit.stripNonEdible(); } Or am I missing something? ---- The nature of this problem has always bothered me - if it was important to retain the identity of the apples as Apples, why on earth did you drop them into a bag of Fruit first? ''That's the real '''core''' of the problem right there! There's a '''design problem'''! But before I judge too harshly, I should confess that in one of the codes bases with which I work, I can think of some places where what we do is like going to the grocery store, picking out the good fruit, having it put into a bag at check-out and then we go home and CoreTheApples. But I always feel weird when I see an 'instanceof' in code. --EricHerman'' That said, is there any reason not to simply turn the whole mess into fruit cocktail using the Acyclic Visitor of Doom? ---- class Fruit implements Sliceable {} class Apple extends Fruit implements Coreable {} class Banana extends Fruit implements Peelable {} while (fruitBag.hasNext()) { Fruit fruit = (Fruit) fruitBag.next(); if (fruit instanceof Coreable) { ((Coreable)fruit).core(); } else if (fruit instanceof Peelable) { ((Peelable)fruit).peel(); } addToBowl(fruit.slice()); } Any idea how to remove the if-elseif blocks? I can't think of anything as they are different operations on a fruit. -- VhIndukumar ''Remove the if-elseif blocks by abstracting core and peel to a common operation, say prepare(), then have the subclasses of Fruit implement that common operation as appropriate, or implement an interface with some sort of Preparer object.'' The problem with this is that these are different operations. A prepare() operation is not possible. Because, A fruit can be peelable and also coreable. And they have to be treated separatly. So these are different types of operations. They do not come under a particular hierarchy. -- VhIndukumar Also, it's worth noting that some fruits need to be peeled to be eaten (bananas), and some fruits are peeling-optional (apples). So do we need a separate interface, or static boolean value PEEL_OPTIONAL? I guess this is the problem with writing code when you don't know exactly what it needs to do. ''This demonstrates the niceness of having context-sensitive software design, and why frameworks tend towards evil. Prepare() is more than sufficient in the above example. Examine the code above closely; if the fruit is both corable ''and'' peelable, coring takes priority. Hence, the implementation of prepare() would choose to core, not peel(). Don't complexify the problem! --SamuelFalvo'' ---- public interface Fruit { public abstract void accept(FruitVisitor visitor); } public class FruitVisitor { public void visitApple(Apple apple) {} public void visitBanana(Banana banana) {} } public class Apple implements Fruit { public void accept(FruitVisitor visitor) { visitor.visitApple(this); } public void core() { ... } } public class Banana implmements Fruit { public void accept(FruitVisitor visitor) { visitor.visitBanana(this); } } public class AppleCorer extends FruitVisitor { private static FruitVitisor _visitor = new FruitVisitor() { public void visitApple(Apple apple) { apple.core(); } } public static void coreApples(Iterator i) { while (i.hasNext()) { ((Apple) i.next()).accept(_visitor) } } } --JasonArhart This only buries and hides the '''instanceof''' in '''Fruit.visit()''', but using a lot more code, memory, and execution time to do it. -- SunirShah I don't disagree that it's an overly complex solution to a simple problem. If somebody suggested this as the best solution to the problem I'd recommend awarding a PropellerBeanie. I was simply adding a solution to the list, as if brainstorming. I don't think it hides the '''instanceof''' however. I think it removes it completely. Instead of checking for a particular class, it effectively asks the fruit if it's an apple. It's based on behavior, not class. In reality I would probably solve it like this: Iterator i = fruitBag.getApples(); while (i.hasNext()) { coredApples.add(core((Apple) i.next())); } Or in RubyLanguage: cored_apples = fruit_bag.apples.collect { |apple| core(apple) } After all, why bother examining every fruit if you only care about the apples? Let the fruit bag worry about separating (or merely keeping separate) the apples from the rest of the fruit. --JasonArhart Sorry. I didn't mean to criticize you, merely the solution if anyone chose to use it. ''No apology necessary.'' You're right, I bungled. There is no '''instanceof''' being hidden because there is no '''instanceof'''. -- SunirShah ---- '''Fault-tolerant DynamicDispatch''' As someone mentioned above, the problem is that the collection is heterogenous when you want to preserve type information. One answer was to abstract the operation you want to perform so that all types of Fruit respond, but this is wrong because we need only invent operations that we want to perform on a particular subclass, ad nauseum, until the base class responds to every verb in the English language. Shall we '''candy''' a Tomato? Here's another approach, available in dynamic languages: '''for each''' fruit in bag perform core on fruit; '''if''' message not understood '''then''' do nothing. '''endfor''' Is this better than regaining the type information by querying the objects for it? Maybe, but consider that most virtual machines these days will replace the dynamic look up with '''if''' fruit '''instanceof''' Apple '''then''' perform Apple>>core on fruit '''else''' perform core on fruit ''(if there is another responder to core)'' '''endif'''; '''if''' message not understood '''then''' do nothing. But it's certainly no worse than moving '''core''' (or '''prepare''') up to the base class because you still have to do the dynamic look up, except then ''every'' class responds instead of just the one you want. -- SunirShah ---- '''Exceptions''' Another way to pretend that you aren't doing an '''instanceof''' is to abuse the exception handlers: for( ... ) { // Access violations are good programming? try { dynamic_cast(pFruit)->core() } catch( ... ) {} } or for( ... ) { try { ((Apple)fruit).core() } catch ( ClassCastException e ) {} } But this is really inefficient. -- SunirShah ---- '''ParametricPolymorphism''' If each recipe prepares the fruits in a different way (squeeze an orange?), then you can use parametric polymorphism. Fruit '''ISA''' Object. Apple '''ISA''' Fruit. Orange '''ISA''' Fruit. Plum '''ISA''' Fruit. '''class''' Recipe ''"Most general match"'' prepare( Object o ) { ''"do nothing"'' } '''endclass''' '''class''' F''''''ruitSalad '''ISA''' Recipe ''"When it doubt, if it's a fruit, wash it."'' prepare( Fruit fruit ) { fruit.wash(); } ''(covers Plum case)'' ''"Concrete matches"'' prepare( Apple apple ) { apple.wash(); apple.core() } prepare( Orange orange ) { orange.peel(); orange.squeeze() } '''endclass''' recipe <-- new F''''''ruitSalad '''foreach''' fruit '''in''' fruits '''do''' recipe.prepare(fruit) '''endfor''' This is not possible unless the DynamicDispatch includes a type-hierarchy aware unifier. (Zzz..) -- SunirShah ---- With just about any fruit, there's going to be some portion you'd want to remove before using the fruit in a recipe, no? So how 'bout the interface for Fruit (parent class of Apple, Banana, etc.) includes a function Remove''''''Unused''''''Portion(), which up resolving to a call to Apple::Core() (or perhaps calls to Apple::Peel ''and'' Apple::Core), Banana::Peel(), Cucumber::Bisect() then Cucumber::Remove''''''Seeds(), etc.? When preparing a recipe, you just call Remove''''''Unused''''''Portion() on all Fruits. Am I missing something here? // interface class for fruits struct IFruit { // to be implemented as required for each fruit virtual void RemoveUnusedPortion() = 0; // other common fruit operations also defined as pure virtual ... }; // Apple - a specific fruit class Apple : public IFruit { public: // removing the unused portion of an apple means peeling and coring it void Remove''''''Unused''''''Portion() {Peel(); Core();} // other fruit action implementations ... private: // functions required to implement Remove''''''Unused''''''Portion() void Peel() {/* whatever is done to peel an Apple*/} void Core() {/* whatever is done to core an Apple*/} }; --MikeSmith The third answer was to add '''prepare()''' (which was reiterated later on the page). Someone else suggested '''stripNonEdible()'''. Now we have '''R''''''emoveUnusedPortion()'''. Changing the name of the method doesn't make the solution any different, nor better. -- SunirShah OK, so I missed the stripNonEdible() one. prepare() is different in that "preparing" a fruit can involve additional steps beyond the removal of unused portions - such as slicing into smaller pieces, which may be required with some fruits but not others. So what is ''wrong'' with stripNonEdible() or R''''''emoveUnusedPortion() as a solution? Assuming, of course, that you have to live in the real world and Smalltalk is not an available option? Why should the recipe-processing code have to know that Apples get Cored, while Bananas get Peeled? Isn't it better if the recipe-processing code only needs to know that Fruits (''all'' Fruits) get stripNonEdibled? In my opinion, this approach is optimally general while being less complex than the Visitor approach, and being practical to implement in the non-dynamic languages that many of us barbarians have to deal with. I should clarify - it is optimal ''for this specific problem''. If you're looking for a different solution, pose a different problem. --MikeSmith The phrase '''stripNonEdible''' is not only clumsy, but it doesn't really describe what we are doing. Since the ''abstract'' goal is preparation of the ''fruit'', we '''prepare''' each ''Fruit''. When we '''prepare''' each ''Apple'', we '''core''' each Apple. We may also want to '''wash''' the Apple. We may not want to '''stripNonEdible''' the Plum. That's why we call the abstract method in Fruit '''prepare'''. Fruit>>prepare '''self''' implementedBySubclass ! Apple>>prepare '''self''' wash; core. ! Plum>>prepare '''self''' wash. ! Orange>>prepare '''self''' peel; split. !! Second, in the RealWorld, I would write '''eval { $_->core } foreach @fruits;''' :-) -- SunirShah ---- ''With just about any fruit, there's going to be some portion you'd want to remove before using the fruit in a recipe, no? [So] When preparing a recipe, you just call Remove''''''Unused''''''Portion() on all Fruits. Am I missing something here? '' 1. unusedPortion is a dreadful name. It sounds like it should return whatever was left over when you finished. extractFlesh would be better, though it still isn't clear if extractFlesh modifies the object or returns a new object. (Remember - side effects are evil.) ''But unusedPortion wasn't the name - the name was R''''''emoveUnusedPortion: it removes the unused portion and discards it.'' 2. and far more important, different recipes will have a different unusedPortion. Eg, What should Egg::extractFlesh() do? Return the white, the yellow, or both? ---- Simple answer in GenericHaskell: gmap (mkT core) bag More complex answer: how stupid to throw all sort of fruit together. Apples should be cored before getting mixed up with other fruit.