views:

128

answers:

5

Hi.

Let's say I've got a type called Superstar. Now I want to have a method that does some work and edits some properties of a Superstar object.

Here are two ways of how I could implement this. Way 1 would be the following:

private Superstar editSuperstar(Superstar superstar){
    ....
    superstar.setEdited(true);
    return superstar;
}
...
superstar = editSuperstar(superstar);

And way 2 would be this:

private void editSuperstar(Superstar superstar){
    ....
    superstar.setEdited(true);
}
...
editSuperstar(superstar);

Which one of these two possible ways is considered "best practice"? The first one, or the second pseudo "by reference" one?

+5  A: 

In your case, the second form is preferrable, as you directly change one of you superstar properties (edited). However, if you have a method that use a superstar object and returns an updated version of it (without changing the initial one) the first form will have my favor.

Finally, since both of this examples only use Superstar object, they should be member methods of the Superstar class.

Riduidel
+4  A: 

Use way 2 unless you are creating a "builder" class where you intend to chain invocations. Ex:

MyClass c = (new MyClassBuilder()).setX(blah).setY(blah).build();
Michael Aaron Safyan
Your example is quite different, it rather illustrates the idea of returning "this" -- instead of "void" -- from a setter method to allow chaining setter invocations...
pgras
+2  A: 

The first form is deceptive. It gives the impression that one object is being passed in, which is copied and the copy then altered and returned.

"Best practice" would be to use the first form, but to actually do what is implied (apply the change to a copy, which is then returned). Immutable objects should generally be preferred over mutable objects unless they are big chunky things that are expensive to copy, in which case, you should then favor the second form.

Marcelo Cantos
+2  A: 

The problem you have here with first method is if you use it like that:

Superstar edited = editSuperstar(originalSuperstar);

This will also modify the originalSuperstar which is, in my opinion, counterintuitive...

Thus prefer the second one if you modify the passed object or the first one if you return a new copy of the object.

For this peculiar example you could simply add an edit method to the Superstar class...

pgras
A: 

The first form is the one to go, i want to know what methods do with my arguments. otherwise i would have to hand over defensive copies to be shure nothing gets. But you have to make a defensive copy, change it, and then return it. Dont change input parameters by reference. Dont do that. If you do that, make that clear in the docs. I probably would make Superstart immutable, by the way.

Have a look at Effective Java by Josh Bloch, there are many good hints about topics like this one (and a few exactly about this topic).

atamanroman