Core The Apples

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 DoNothing() 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 FruitSalad(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 ApplePie(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 ApplePie(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 Apples and calling core() on them. What is the importance of the Fruit class if all we care about are Apples? Why not just iterate through a list of Objects?

 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<Apple*>(pFruit)->core() } catch( ... ) {} 
    }

    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 FruitSalad 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 FruitSalad

    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 RemoveUnusedPortion(), which up resolving to a call to Apple::Core() (or perhaps calls to Apple::Peel and Apple::Core), Banana::Peel(), Cucumber::Bisect() then Cucumber::RemoveSeeds(), etc.? When preparing a recipe, you just call RemoveUnusedPortion() 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 RemoveUnusedPortion() {Peel(); Core();}

   // other fruit action implementations
   ...

 private:
   // functions required to implement RemoveUnusedPortion()
   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 RemoveUnusedPortion().

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 RemoveUnusedPortion() 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 RemoveUnusedPortion() 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 RemoveUnusedPortion: 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.