Is it good practice in Java for a class's method to redundantly return a modified global field of the class?

Question

Is it good practice in Java for a class's method to redundantly return a modified global field of that class?

Note: I edited the examples below to correct my code.

For example:

public class MyClass {
    private String veryImportantString;

    public static void main(String [] args) {
        MyClass myObject = new MyClass();
        myObject.fieldQuestion();
    }

    public void fieldQuestion() {
        veryImportantString = "Hello, StackOverflow";
        String newString = addStringToVeryImportantString(" World!");
        System.out.println(newString);      
    }

    public String addStringToVeryImportantString(String inputStr) {
        this.veryImportantString = veryImportantString + inputStr;

        return veryImportantString;
    }
}

In the above example, addStringToVeryImportantString is returning veryImportantString, even though as a class field it has been modified and is globally available to the calling method.

In the below example, I do the same thing without returning the field:

public class MyClass {
    private String veryImportantString;

    public static void main(String [] args) {
        MyClass myObject = new MyClass();
        myObject.fieldQuestion();
    }

    public void fieldQuestion() {
        veryImportantString = "Hello, StackOverflow";
        addStringToVeryImportantString(" World!");            
        String newString = veryImportantString;
        System.out.println(newString);
    }

    public void addStringToVeryImportantString(String inputStr) {
        this.veryImportantString = veryImportantString + inputStr;
    }
}

My question is: does it matter? Is there any difference in terms of coding standards, readability, efficiency, etc.? In a way, it makes sense to never return a global class field that a function has modified, because what is the point? The field is available anyway. On the other hand, maybe it makes sense to return the field in order to indicate that the primary purpose of the function is to modify that field and return the value.

Thanks for your input.

Your second example wouldn't even compile. You can't access instance variables from a static context. — Brian Roach, Aug 10 '13 at 22:46
Unless theres a specific purpose then its best to avoid creating confusing method signatures. As a rule (that I break when appropriate but only after thinking about it) a method should either change the state of some object (or variable) or get information on its current state but never both. — Richard Tingle, Aug 10 '13 at 22:48
This isnt a duplicate but is related: [benefits-and-drawbacks-of-method-chaining-and-a-possibility-to-replace-all-void](http://stackoverflow.com/questions/16976150/benefits-and-drawbacks-of-method-chaining-and-a-possibility-to-replace-all-void-parameters-by-the-object-itself) — Richard Tingle, Aug 10 '13 at 22:50
None of this compiles, you can't call non-static methods from main. If you make the methods static, then you can't directly reference instance variable in the methods... — Steve P., Aug 10 '13 at 22:54
This is more or less a poll so my two cents; the first snippet seems strange and actually implies that veryImportant string remains unchanged at first glance (because it returns something). There may be specific reasons to do it but I'd expect to see the second much more regularly — Richard Tingle, Aug 10 '13 at 22:59
Good idea. Best to post real code rather than kind-of sort-of code. — Hovercraft Full Of Eels, Aug 10 '13 at 22:59
Okay, I've edited both examples so that they compile and return output now. Sorry about that ;) -- Richard thank you for your answer; I think the only reason to do it as in the first snippet (which you say is strange) is to make the purpose of the function clear -- in this case the purpose is to modify that field. But that's exactly what I'm looking for, thank you for the input. — emery, Aug 10 '13 at 23:06
Its worth notimg that snippet 2 is unnecessarily elongated. There is no need for newString at all. veryImortamtString could be printed directly. Leading to a program thats the same length as snippet 1 but where you can guess (correctly) the function of addStringTo... without having to see its implementation — Richard Tingle, Aug 10 '13 at 23:11
I really feel it does the opposite; it makes it look like it returns a **new** string thats based upon veryImportantString rather than changing it. Assume it returns void; what could the function reasonably do; only one thing: change veryImportantString. With a return value it could either return a completely seperate string or edit veryImportantString and you have to see the implementation to know which — Richard Tingle, Aug 10 '13 at 23:13
I think that's a cogent and intelligent answer; you should vote the question up and post your opinion as an answer if you have time since the comments are badly polluted with the backlash of my newbish non-compiling code, hehe — emery, Aug 10 '13 at 23:18

score 3 · Answer 1 · answered Aug 10 '13 at 23:29

It is a usual coding standard for a method to either mutate an object (or variable) or get information about it but never both. Therefore a method that has a return will be assumed at first glance not to have any secondary effects (like changing veryImportantString). This is not a hard and fast rule and there are many reasons to break it but it should never be broken lightly.

Secondly the method signature in the second snippet (with a void return) could only reasonably do one thing; change veryImportantString. But with a return it could do two things; change veryImportantString and return it or return a seperate string based upon veryImportantString. As the void signature is less ambiguous it is preferable (unless theres a specific reason to do otherwise)

score 0 · Answer 2 · answered Aug 11 '13 at 00:36

For your example, its not particularly important.

In general when you have a private piece of data and a public method. Your only way to get information out to the user of an instance of your class related to the private data is via returning something from a public method. Remember that most Java programs are much more complex and the need to return data related to a private field (java beans for instance) are much more common.

Is it good practice in Java for a class's method to redundantly return a modified global field of the class?

2 Answers2