Java method convention

Question

There was a basic question I always had in mind. Maybe its too trivial to ask - but I decided to get an opinion anyways.

Heres a sample code:

class seventeenth{

    public static void appendtolist(List<Integer> i){
        i.add(new Random().nextInt(1000));
        i.add(new Random().nextInt(1000));
        i.add(new Random().nextInt(1000));
    }
    public static List<Integer>  returnlist(){
        List<Integer> i=new LinkedList<Integer>();
        i.add(new Random().nextInt(1000));
        i.add(new Random().nextInt(1000));
        i.add(new Random().nextInt(1000));
        return i;
    }
    public static void main(String[] args){
        List<Integer> l=new LinkedList<Integer>();
        appendtolist(l);//Option 1
        l=returnlist();//Option 2
        for(Integer e:l)
        System.out.println(e);
    }

}

Which of the options above is a good programming practice and why?Or it really does not matter? Would appreciate if someone could share any literature around basic good programming conventions like this.

Your class name should start with a capital letter (e.g. Seventeenth). I would call it getList() and appendToList(...) or just add(...). Anyway, find a convention that fits you and stick to it! This way it is easier to read. — Burkhard, Apr 09 '13 at 14:14
@Burkhard The question was about which of option 1 or option 2 is the best — Alexis C., Apr 09 '13 at 14:15
@ZouZou: That's why I just added it as a comment and not as an answer ;) — Burkhard, Apr 09 '13 at 14:16
@Burkhard Ok :) I thought you had misunderstood the question — Alexis C., Apr 09 '13 at 14:18
and always use curly brackets { } with if, for, while etc. Even if you write one-line statement. — Sergii Shevchyk, Apr 09 '13 at 14:23
Both are fine and subject to personal preference. I have a mathematical background and thus like functions that return a single result and do not alter their parameters, so I would vote for option 2. — Vincent van der Weele, Apr 09 '13 at 14:26

score 3 · Answer 1 · answered Apr 09 '13 at 14:15

Both are fine, as long as the names make the function clear.

The "append to" version is a little bit more general, since it can be called several times to append to the same list. Using the "return" version in this manner would require copying everything twice.

I probably would generalize the "append to" version to take any Collection:

public static void addTo(Collection<Integer> coll) {
    coll.add(random.nextInt(1000));
    coll.add(random.nextInt(1000));
    coll.add(random.nextInt(1000));
}

score 2 · Answer 2 · answered Apr 09 '13 at 14:16

One good practice is to reuse objects as much as possible, so instead of

i.add(new Random().nextInt(1000));
i.add(new Random().nextInt(1000));

you would instead use

Random rand = new Random();
i.add(rand.nextInt(1000));
i.add(rand.nextInt(1000));

With the first method you have the overhead of creating and garbage-collecting two Random objects, whereas the second method only creates and garbage-collects one Random object.

score 0 · Answer 3 · answered Apr 09 '13 at 14:15

0

Option 2,

public static List<Integer>  returnlist(){
    List<Integer> i=new LinkedList<Integer>();
    i.add(new Random().nextInt(1000));
    i.add(new Random().nextInt(1000));
    i.add(new Random().nextInt(1000));
    return i;
}

I think is much better, unless you want to go on adding more elements to the list l.

answered Apr 09 '13 at 14:15

The Cat

2,375
6
25
37

It makes the code in the `main` method more maintainable. – The Cat Apr 09 '13 at 14:18
http://en.wikipedia.org/wiki/Factory_method_pattern If this is all your trying to achieve then Option 2 is better. – The Cat Apr 09 '13 at 14:24
This method is tightly bound to LinkedList. It won't return Vector, ArrayList or other list implementation. Better to use first option I guess, because we have to simply pass `list` reference of the implemented object like Vector, ArrayList. [See here my answer](http://stackoverflow.com/a/15904540/1115584) – AmitG Apr 09 '13 at 14:49

score 0 · Answer 4 · answered Apr 09 '13 at 14:20

This depends on meaning of the function, the first one's job is to change a list and the other is to construct one.
I would use method overloading to allow either option without code repition:

public static List<Integer> addRandom(List<Integer> i){
    i.add(new Random().nextInt(1000));
    i.add(new Random().nextInt(1000));
    i.add(new Random().nextInt(1000));
    return i;
}
public static List<Integer> addRandom() {
    return addRandom(new LinkedList<Integer>());
}
public static void main(String[] args){
    List<Integer> l=new LinkedList<Integer>();
    addRandom(l);//Option 1
    l = addRandom();//Option 2
    for(Integer e:l)
    System.out.println(e);
}

Again, they don't have the same meaning but if you really want to compare the two note that the returnList forces the implementation of the List (LinkedList) and the fact

AmitG · Answer 5 · 2013-04-09T14:30:18.193

Using camel case for method names and using pascal case for class name is also good practice.
1> Both your methods are creating new Random() object multiple time. should be avoided.
2> Number generation should not be hard coded like 1000. In future you may require 2000.
3> In returnList method you are returning LinkedList only. But in future you may require it to return Vector, ArrayList. So there is tight bound with the code. It is not flexible.

for the question which one is good

I will always prefer appendToList because I can pass any object in that method like, Vector, ArrayList or LinkedList.

Following modification in method can be useful for good practice.

import java.util.LinkedList;
import java.util.List;
import java.util.Random;

class Seventeenth {

    public static void appendToList(Random random, List<Integer> list, int[] numbers) {
        for (int i : numbers) {
            list.add(random.nextInt(i));
        }
    }

    public static void main(String[] args) {
        List<Integer> list = new LinkedList<Integer>();
        Random random = new Random();
        appendToList(random, list, new int[] { 1, 2, 3, 1000, 2000 });// Option 1
        for (int e : list)
            System.out.println(e);
    }

}

score 0 · Accepted Answer · answered Apr 09 '13 at 14:41

IMO I would prefer the returnlist-kind because I try to avoid to manipulate the parameters of a method since those changes will be "hidden" from the overall flow of the code which could lead to unexpected behaviour later on. However, since the method name is appendtolist it's pretty clear what's about to happen so both approaches are fine to me.

score 0 · Answer 7 · edited May 23 '17 at 12:21

0

It's better NOT TO CHANGE an argument, that's why I would choose the SECOND variant.

Your method should not do anything besides the logic and returning the value here

More about side effects

edited May 23 '17 at 12:21

Community

1
1

answered Apr 09 '13 at 14:42

Sergii Shevchyk

38,716
12
50
61

Java method convention

7 Answers7