4

Which one is recommended considering readability, memory usage, other reasons?

1.

String strSomething1 = someObject.getSomeProperties1();
strSomething1 = doSomeValidation(strSomething1);

String strSomething2 = someObject.getSomeProperties2();
strSomething2 = doSomeValidation(strSomething2);

String strSomeResult = strSomething1 + strSomething2;
someObject.setSomeProperties(strSomeResult);

2.

someObject.setSomeProperties(doSomeValidation(someObject.getSomeProperties1()) + 
                             doSomeValidation(someObject.getSomeProperties2()));

If you would do it some other way, what would that be? Why would you do that way?

8 Answers 8

17

I'd go with:

String strSomething1 = someObject.getSomeProperties1();
String strSomething2 = someObject.getSomeProperties2();

// clean-up spaces
strSomething1 = removeTrailingSpaces(strSomething1);
strSomething2 = removeTrailingSpaces(strSomething2);

someObject.setSomeProperties(strSomething1 + strSomething2);

My personal preference is to organize by action, rather than sequence. I think it just reads better.

12

I would probably go in-between:

String strSomething1 = doSomeValidation(someObject.getSomeProperties1());
String strSomething2 = doSomeValidation(someObject.getSomeProperties2());
someObject.setSomeProperties(strSomething1 + strSomething2);

Option #2 seems like a lot to do in one line. It's readable, but takes a little effort to parse. In option #1, each line is very readable and clear in intent, but the verbosity slows me down when I'm going over it. I'd try to balance brevity and clarity as above, with each line representing a simple "sentence" of code.

6

I prefer the second. You can make it just as readable with a little bit of formatting, without declaring the extra intermediate references.

someObject.setSomeProperties(
    doSomeValidation( someObject.getSomeProperties1() ) + 
    doSomeValidation( someObject.getSomeProperties2() ));

Your method names provide all the explanation needed.

4

Option 2 for readability. I don't see any memory concerns here if the methods only do what their names indicate. I would be vary with concatenations though. Performance definitely takes a beat with increasing string concats because of the immutability of Java Strings.

Just curious to know, did you really write your own removeTrailingSpaces() method or is it just an example ?

1
  • Of course an example.. And I removed it since you might be thinking why my version.. :) Nov 25, 2008 at 15:29
3

I try to have one operation per line. The main reason is this:

setX(getX().getY()+getA().getB())

If you have a NPE here, which method returned null? So I like to have intermediate results in some variable which I can see after the code fell into the strong arms of the debugger and without having to restart!

2

for me, it depends on the context and the surrounding code.

[EDIT: does not make any sense, sorry] if it was in method like "setSomeObjectProperties()", I'd prefer variant 2 but perhaps would create a private method "getProperty(String name)" which removes the trailing spaces if removing the spaces is not an important operation [/EDIT]

If validation the properties is an important step of your method, then I'd call the method "setValidatedProperties()" and would prefer a variant of your first suggestion:

validatedProp1 = doValidation(someObject.getSomeProperty1());
validatedProp2 = doValidation(someObject.getSomeProperty2());
someObject.setSomeProperties(validatedProp1, validatedProp2);

If validation is not something important of this method (e.g. there's no point in returning properties which are not validated), I'd try to put the validation-step in "getSomePropertyX()"

1

Personally, I prefer the second one. It's less cluttered and I don't have to keep track of those temporary variables.

Might change easily with more complex expressions, though.

1

I like both Greg and Bill versions, I think I would more naturally write code like Greg's one. One advantage with intermediary variables: it is easier to debug (in the general case).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.