Programming Practices – How to Check if a Method Returns False

clean codecoding-styleobject-orientedprogramming practices

Is it a good practice to call a method that returns true or false values in an if statement?

Something like this:

private void VerifyAccount()
{
    if (!ValidateCredentials(txtUser.Text, txtPassword.Text))
    {
        MessageBox.Show("Invalid user name or password");
    }
}

private bool ValidateCredentials(string userName, string password)
{
    string existingPassword = GetUserPassword(userName);
    if (existingPassword == null)
        return false;

    var hasher = new Hasher { SaltSize = 16 };
    bool passwordsMatch = hasher.CompareStringToHash(password, existingPassword);

    return passwordsMatch;
}

or is it better to store them in a variable then compare them using if else values like this

bool validate = ValidateCredentials(txtUser.Text, txtPassword.Text);
if(validate == false){
    //Do something
}

I am not only referring to .NET, I am referring to the question in all programming languages it just so happens that I used .NET as an example

Best Answer

As with all these things it depends.

If you aren't going to use the result of your call to ValidateCredentials then there's no need (other than for debugging purposes) to store the result in a local variable. However, if it makes the code more readable (and hence more maintainable) to have a variable go with that.

The code isn't going to be measurably less efficient.

Related Solutions

Java – if i have many calls of single method that returns field value, is it better to make a local variable

For the sake of clarity, I would suggest using final Field value = foo.value() (final since you specify the value doesn't change) if you have more than a single method call. Performance wise there will be no difference, the JIT compiler will likely completely inline all the calls to a simple accessor method.

There are a few reasons to prefer this: most important is that this shows whoever is reading the code that you are definitely using the same variable, and it definitely does not change (by use of final). This is more clear than using multiple methods calls - whoever is reading it will need to chase down if it really is using the same thing each time you call a method. Less importantly, if the method name changes, it only needs to change in one spot here, instead of needing to modify it at each point where it is used.

Java – Is throwing an exception an anti-pattern here

Yes, your colleague is right: that is bad code. If an error can be handled locally, then it should be handled immediately. An exception should not be thrown and then handled immediately.

This is much cleaner then your version (the getValueByKey() method is removed) :

public String getByKey(String key) {
    if (valuesFromDatabase.containsKey(key)) {
        return valuesFromDatabase.get(key);
    } else {
        String defaultValue = defaultValues.get(key);
        valuesFromDatabase.put(key, defaultvalue);
        return defaultValue;
    }
}

An exception should be thrown only if you do not know how to resolve the error locally.

Best Answer

Related Solutions

Java – if i have many calls of single method that returns field value, is it better to make a local variable

Java – Is throwing an exception an anti-pattern here

Related Topic