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.

Code Complete (2nd ed.) says:

"If you pass a parameter to a routine, use it. If you aren't using it, remove the parameters from the routine interface."

However, in the past I sometime had to do that. Consider the following example:

Assume that we have an abstract base class called Food, containing a pure virtual function called getEaten. We derive two separate classes from it:

class Apple : public Food
{
    void getEaten(const Utensil &utensil);
    // otherstuff
};

and

class Soup : public Food
{
    void getEaten(const Utensil &utensil);
    // otherstuff
};

When we call mySoup.getEaten(spoon), the function makes use of the spoon parameter in order to perform the action. However, an apple does not need a utensil, and in those cases I have to send a dummy utensil to myApple.getEaten(emptyUtensil).

Yes, this is ugly, and has the overhead of unnecessarily creating and passing a dummy object. I can generalize the problem like this:

"When we have two concepts, which are represented as two classes, have the same action applied on one of them, and one needs more information than the other when requesting to execute those actions, I tend to pass dummy information to the less information requesting objects."

So my questions are:

1) How can we change the design of this somewhat silly example, so that we don't need to pass the dummy utensil?

2) If there is a nice simple solution to the first question, would there be a way to generalize it to the generalized problem above (like solutions as "in those cases create a new x such that it interacts with the less informating demanding class like this" etc.)

share|improve this question

6 Answers 6

up vote 4 down vote accepted

The example is a bit silly, but going along with it, one could argue one of three cases:

  1. The action 'eat' is in fact a compound action that needs to be broken down into steps (cut, pick up, put in mouth, chew, swallow). The required steps differ per kind of edible; depending on whether you're a food.beEaten(eater) or a eater.eat(food) type. The eater would have to know which steps to take for each kind of food (probably using an EatingProcedureFactory to determine the correct IEatingProcedure for the food in question). The eating procedure in turn would have methods to determine which utensils are required (or, more broadly, a method to check whether its requirements are met).
  2. In food.beEaten(eater), check if the eater has all the required utensils.
  3. Be pragmatic and leave it that way. This is not worth going 100%-correct-OOP, the benefit is small and doesn't outweigh the added complexity.
share|improve this answer

IMHO, you're taking the rule

If you pass a parameter to a routine, use it. If you aren't using it, remove the parameters from the routine interface

just too far. The interface is common to both Apple and Soup, so it requires an Utensil and there's nothing sensible you could do about it. The fact that Apple::getEaten doesn't actually need it is an implementation detail.

When calling Food::getEaten you simple have to provide an Utensil and that's fine. You may provide a parameterless method Apple::getEaten() in case you need to call it directly and have no Utensil. You most probably should not promote this method to the superclass.

Of course, using List<Utensil> is the right solution in case more utensils may be needed.

share|improve this answer
    
Rationalizing this - you could argue that there's one method which has several implementations, plus a layer of run-time dispatch implementation to do the choosing. The fact that a run-time dispatch chooses isn't that different to some other choice mechanism, such as a switch. If any part of the implementation requires the parameter then the whole method requires the parameter, just as if one case of a switch requires the parameter then the whole switch requires the parameter. –  Steve314 Aug 6 '11 at 18:37

Why not use the Strategy pattern?

The example given is written in Java (I pretty much suck at C++) but should give you an idea:

public interface Utensil {
  void apply();
}

public class Spoon implements Utensil {
   public void apply() {
       System.out.println("Using spoon...");
   }
}

public class Food {
    private Utensil utensil;

    public Food(Utensil utensil) {
        this.utensil = utensil;
    }

    public void getEaten() {
        utensil.apply();
    }
}

public class Apple extends Food {
    public Apple() {
        super(new Spoon());
    }
}
share|improve this answer

This example does have an obvious solution though. Consider eating a steak: you'd probably use a knife and fork. Soup: spoon. Apple: nothing. So, in the most general case of meals, you need a list of utensils, not a single utensil.

The list, then, could be empty in the case of an Apple.

share|improve this answer
    
You right, the example is a bit crappy. Please assume that the extra information needed by the one of the classes is something which should be represented by one of the existing classes. –  loudandclear Jul 21 '11 at 8:40
    
I would have to argue that most cases of "optional" arguments can be solved by passing an argument list, which may be empty. There is also the possibility of allowing the argument to be NULL, and checking for that. –  wolfgangsz Jul 21 '11 at 8:55

In answer to your first question what you can do will depend on what language you're using. As a general OOP solution you can use polymorphism to provide a second implementation of getEaten which does not need a utensil.

//Examples are converted to c#
class Food
{
    public virtual void getEaten(Utensil utensil);
    public virtual void getEaten();
}

class Soup :  Food
{
    public void getEaten(Utensil utensil)
    {
        // consume the soup
    }
    public void getEaten()
    {
        throw new Exception("A spoon is required"); 
    }  

}

Depending on the language you are using and the type you're passing in the function then another option would be an optional parameter with a default value.

class Soup : Food
{
    // C# only supports compile-time constants as default values
    // so the above example using a Utensil class wouldn't work.
    void getEaten(string utensilType = "Spoon");
    {
        // consume the soup
    }
}

As a final option you could just have the function support being passed null, as in no utensil is required.

Off these I would suggest the first. It also has the advantage of supporting additional getEaten methods may take two or more utensils.

share|improve this answer
    
For an implementation with support for n utensils, populating each class with n unimplemented/exception-throwing member functions also seemed like bad practice (even for n=1), not sure though. However, yes, that would be a lot better than the original design. –  loudandclear Jul 21 '11 at 8:37
    
In your instance I would probably opt for something along the lines of: –  CdMnky Jul 25 '11 at 8:19
    
getEaten(List<Utensil> utensils) where utensils may contain 0 to n indivdual utensils. –  CdMnky Jul 25 '11 at 8:20
    
What you did is far worse than using a single method ignoring its argument. –  maaartinus Aug 6 '11 at 14:45
    
@maaartinus Care to provide some explanation? –  CdMnky Aug 8 '11 at 11:44

Could the Soup class have an overload of getEaten that takes the parameter, whilst the basic Food class has the method signature without the parameter?

interface IFood
{
    void getEaten();
}

class Apple : IFood
{
    void getEaten()
    { /*Stuff */ }
}

class Soup : IFood
{
    void getEaten()
    {
        throw NotImplementedException();
    }

    void getEaten(Utensil utensil)
    { /* Stuff */ }
}
share|improve this answer
    
Throwing NotImplementedException looks pretty much like a code smell. –  Oliver Weiler Jul 21 '11 at 8:48
    
Yeah, it isn't ideal, but was really just there to show that the method shouldn't really do anything. You could quite as easily call the overload with a default value from there. –  AndyBursh Jul 21 '11 at 9:02

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.