I have to set the value of a string object "result" depending on the results of different methods and different if/else conditions. In the end, there would be one (last value set) in the string that i need to use. The first way i did it is:
public void MyClass(){
public void validateApples() {
String result = "failure";
if (condition 1) {
//do something
}
if (condition 2) {
//do something
}
if(condition 3) {
//do something
}
if (condition 4) {
if (condition a)
result = "success";
} else{
//One of the first three conditions resulted in failure so don't change string value
}
}
if (condition 5) {
//Do validation only for xml files
result = validateMachintoshApples ();
}
}
private String validateMachintoshApples() {
String result;
result = oneStepValidation();
return result;
}
private String oneStepValidation(){
String result;
if (some condition) {
// some code
result = "success";
}else{
// some code
result = "failure";
}
return result;
}
}
I feel that i am recreating a new String object many times in different methods of the same class. I don't think its efficient in terms of memory, object creation time and coding standards.
I than moved on to creating one string object "result" for the whole class and assign it "failure" or "success" as i think that would be more clean code.(see code below)
1) But since Stings are immutable in JAVA, will I achieve something this way?
2) Am i still creating new String each time with result = "failure" or result = "success" or is it only a value change? I doubt it is a value change as strings are immutable.
3) Is the second way less readable since the functions are setting the value for result but not returning anything?
4) What should be the best practice in this case?
My second way of coding looks like:
public void MyClass(){
private String result;
public String getResult() {
return result;
}
public void setResult(String result) {
this.result = result;
}
public void validateApples() {
if (condition 1) {
//do something
result = "failure";
}
if (condition 2) {
//do something
result = "failure";
}
if(condition 3) {
//do something
result = "failure";
}
if (condition 4) {
if (condition a)
result = "success";
} else{
//One of the first three conditions resulted in failure
}
}
if (condition 5) {
//Do validation only for xml files
validateMachintoshApples ();
}
}
private void validateMachintoshApples() {
result = oneStepValidation();
}
private void oneStepValidation(){
if (some condition) {
// some code
result = "success";
}else{
// some code
result = "failure";
}
}
}
Best Answer
Let's look at this for a minute or two...
So you are running some code and expect a string return to tell you how it went. In doing this you noticed (and rightly so) that this evokes a feeling somewhat unpleasant, this just dont feel right you say. The issue however is not one of performance, or rather the performance is but a twig that hides the creepy forest behind.
First change you can do here is to replace the string values you just hard code everywhere with constants. This will cover cases where you make typoes assigning the value leading to the cases not being detected by the caller.
But it still feels wrong no ?
Well, you can help yourself a bit more by helping the compiler. Put all the possible values in an enum and assign that instead. Here we are no longer working with strings directly but actual object-like with a type and a strictly controlled set of values. No typos possible and makes it easier to ensure you cover all the possible return values (in cases where you get more than just "success" or "failure")
If you only ever need two cases then a boolean would do just dandy, true = ok, false = fail...
Yet you can still do better here...
enter Exceptions
All this code is basically to check that something is what it's supposed to be right ? So you basically have a success case and one or more failure types.
so instead of returning a value that needs to be checked don't return anything. IF all goes well the code executes normally. If the validation failed then the failure will throw an exception you will have created (ValidationException or something similar) that will be caught by a caller code, could be the direct caller, could be higher up the stack too. It basically will be caught at a level where something can be done about it.
Need more validation types ? (just like the enum above) create sub-classes of this ValidationException for each type of problems that needs detection. This will unclutter your code that deals with the normal case while giving you the means to aggregate the handling code where it matters.
Long story short, the language offers powerful tools to handle errors like this, embrace them. You will find your code much easier to read and maintain.