Sign in
Log inSign up

Having some trouble with object oriented paradigm, advice?

Mark's photo
Mark
·Apr 5, 2018

When developing in Java, I often find that my code doesn't match existing code. I think I may be doing object oriented stuff wrong?

class SaladMaker {
    private List<Fruit> fruitSalad;

    /**
     * Given a recipe, create a fruit salad in {@link fruitSalad}.
     *
     * @param recipe Describe what does in this fruit salad.
     */
    public void makeFruitSalad(Recipe recipe) {
        // Stuff to determines which fruits go in the salad.
        fruitSalad = new ArrayList<>();
        for (Fruit fruit : recipe) {
            if (!addFruitToSalad(fruit)) {
                System.out.println("skipped fruit: " + fruit);
            }
        }
        // There's more stuff here, dressings and such.
    }

    /**
     * Prepare a fruit and add it to the salad.
     *
     * @return Return code indicates success/failure.
     */
    private boolean addFruitToSalad(Fruit fruit) {
        fruit.wash();
        fruit.cut();
        // Let's pretend there's enough stuff here to make a method.
        // And something can go wrong that returns false.
        fruitSalad.add(fruit);
        return true;
    }

    public List<Fruit> getSalad() {
        return fruitSalad;
    }
}

// Used like this: 

SaladMaker saladMaker = new SaladMaker();
saladMaker.makeFruitSalad(new Recipe(/* ... */));
saladMaker.getSalad();

Obviously it is not a real example. Some things to note:

  • The salad maker has a field that contains the salad that everything uses.
  • Return values are not used very much.

In the real example, there may be more methods that are called in a specific order to manipulate those fields.

What I feel tempted to write is more like this:

class SaladMaker {
    /**
     * Given a recipe, create a fruit salad.
     *
     * @param recipe Describe what does in this fruit salad.
     * @return A list of the salad components (mix well!).
     */
    public List<Fruit> makeFruitSalad(Recipe recipe) {
        // Stuff to determines which fruits go in the salad.
        List<Fruit> fruitSalad = new ArrayList<>();
        for (Fruit fruit : recipe) {
            Optional<Fruit> prepared = prepareFruitForSalad(fruit);
            if (prepared.isPresent()) {
                fruitSalad.add(prepared.get());
            } else {
                System.out.println("skipped fruit: " + fruit);
            }
        }
        // There's more stuff here, dressings and such.
        return fruitSalad;
    }

    /**
     * Prepare a fruit.
     *
     * @return The fruit if successful, empty if not.
     */
    private Optional<Fruit> prepareFruitForSalad(Fruit fruit) {
        // Perhaps clone the fruit.
        fruit.wash();
        fruit.cut();
        // Let's pretend there's enough stuff here to make a method.
        // And something can go wrong that returns Optional.empty().
        return Optional.of(fruit);
    }
}

// Used like this:

SaladMaker saladMaker = new SaladMaker();
List<Fruit> salad = saladMaker.makeFruitSalad(new Recipe(/* ... */));

Which is somewhat different:

  • There is no field.
  • It uses return values for data used by multiple methods.

Note that with more data to be communicated, a lot of boilerplate appears, since there is no good multiple-return in Java (Tuple/Triple, but meh).

I like my code for some reasons (obviously, or I wouldn't write it):

  • It's impossible to get the order of methods wrong, since you need the result of the previous one (if you don't, the order isn't wrong).
  • It is immediately obvious what a method needs and produces, tt's hard to call any method without meeting the preconditions.
  • It's easy to reuse and test methods in isolation.
  • If the class grows (some are ten thousand lines), the fields are pretty much like globals.

(It's actually not because I'm new; I used to write first style, but I'm liking return values more and more).

I feel like we have fewer bugs this way. But there are things wrong with it:

  • It's not very Object Oriented. In the example, why have an instance at all? There is no state (in real examples there would be state, just less).
  • The return values can get unwieldy, partly because of Java (in Python there's multiple return, in Kotlin there's one-line data classes, ...).
  • Everyone else working on the codebase uses the normal style, so it probably makes sense (although I'm always open to the idea that I'm the smartest person around, of course).

What is your opinion? Am I doing object oriented programming wrong by trying to be functional? Am I doing programming wrong all together? Is there a third way?

Please share your wisdom!