Skip to content
Advertisement

A global variable as a method argument in the same java class: Is this bad programming practice?

Below code works just fine. But as a newbie I’m not sure if it’s bad practice to pass the global variables into the private method as an argument in the same java class. Is this acceptable? Is there a better or more acceptable way to do it, in case this is bad programming?

@Value("${canDeleteAllBefore:true}")
private boolean canDeleteAllBefore;
@Value("${canDeleteAllAfter:false}")
private boolean canDeleteAllAfter;
@Value("${canExecute:true}")
private boolean canExecute;   
public ConsoleApp(@Value("${canDeleteAllBefore:true}") String canDeleteAllBefore,
                             @Value("${canDeleteAllAfter:true}") String canDeleteAllAfter,
                             @Value("${canExecute:true}") String canExecute) {
   
    setboolean(canDeleteAllBefore, "canDeleteAllBefore", this.canDeleteAllBefore);
    setboolean(canDeleteAllAfter, "canDeleteAllAfter",this.canDeleteAllAfter);
    setboolean(canExecute, "canExecute",this.canExecute);
}
private void setboolean(String booleanValue, String propertyName, boolean boolValue) {
    if (booleanValue.equalsIgnoreCase("true") || booleanValue.equalsIgnoreCase("false")) {
        boolValue = Boolean.parseBoolean(booleanValue);
    } 

    else {
        System.out.println(booleanValue + " is not a boolean value. " +
                "Please use either true or false as a value for the property " + propertyName);
        System.exit(1);
    }
   }

Advertisement

Answer

If the expectancy is that the setboolean method will change the value of the canExecute field, then that’s unfortunately not correct.

So when you call this line

setboolean(canExecute, "canExecute",this.canExecute)

It will actually take the current value of this.canExecute and will pass it in. The method setboolean does not know where that value comes from.

So when setboolean updates the value of the parameter, it won’t have any effect outside the scope of that function.

So, in your case, what would make more sense is:

private boolean setboolean(String booleanValue, String propertyName, boolean boolValue) {
    if (booleanValue.equalsIgnoreCase("true") || booleanValue.equalsIgnoreCase("false")) {
        return Boolean.parseBoolean(booleanValue);
    } 
    throw new Exception("invalid value");
}

And then use the function like this.canExecute = setboolean(...); Just using the return value of the function.

There is another way. You may want to look at BooleanHolder, which acts like a wrapper around a primitive boolean. The advantage is that you can pass it around and change the content of the holder. That’s close to what you attempted to do.

User contributions licensed under: CC BY-SA
10 People found this is helpful
Advertisement