Is it bad to have magic variables?

Say I have a variable or object I want to pass from one class to another through a te开发者_Go百科st class. Let's say I have a test class, a chef class, and a waiter class.

Is it bad to do this in the test class:

chef.makeFood();
waiter.deliverFood(chef.getFood())

On that same note, should I instead do this:

chef.makeFood();
Food f = chef.getFood();
waiter.deliverFood(f);

Is it bad to have magic variables?

It depends what you mean by "magic". If "magic" means to you "does something that is hard to understand or explain", then it is bad.

Is it bad to do this in the test class:

  chef.makeFood();
  waiter.deliverFood(chef.getFood())

I don't see anything wrong with that code per se, even if that is not how the classes are used in normal (non-test) code. (But there is a problem in the design/code that you are not showing us ...)

On that same note, should I instead do this:

  chef.makeFood();
  Food f = chef.getFood();
  waiter.deliverFood(f);

It is OK to do that, or not to do that. It depends on whether you think that makes the code more readable. (Personally, I wouldn't bother with a local variable there, but that's just my opinion.)

On the other hand, I agree with @DaveNewton's comment about the "chef" object holding the food in "makeFood". This is certainly counter-intuitive and a bit "magic" ... if it was a deliberate design choice.

A simpler and better design would be this:

Food f = chef.makeFood();
waiter.deliverFood(f);

and entirely remove the getFood() method and the associated state variable from the Chef class.

There are a number of reasons why this is poor design / bad modelling:

• In the intuitive sense, it means that the chef object has to "stand around holding the plate" until the waiter takes it.

• From the class modelling perspective, you have a state variable that that is not necessary to the implement the Chef's behaviour.

• From the practical programming perspective, that means that:

  • the code will be hard(er) to understand when you look at it in 12 months time, and

  • the Chef API will be difficult to use in a multi-threaded kitchen simulation.

(If this is what you meant by a "magic variable", then it is bad ... see above.)

Well this isn't a magic variable or anything of the sort, instead here:

chef.makeFood();
waiter.deliverFood(chef.getFood())

The first one requires less lines, and arguably could be better if you do not need the reference to the newly created Food object, However many could argue that even if you don't need the reference, the code below is more readable and clear:

chef.makeFood();
Food f = chef.getFood();
waiter.deliverFood(f);

In general you should always try to go with the more readable code, because you never know who will be maintaining that code later on.

Is hard to generalize on this example.

In this simple example, there is probably not a reason for chef to keep a reference to `makeFood()', so it should be returned directly. (And then getFood() can be removed)

But there are plenty of other scenarios where it The Right Thing to keep an internal state, like for instance a StringBuilder - or more complex "builder" , and then return the result when its fine.

The local variable usually ends up there when debugging.... It doesn't really matter.