Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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

share|improve this question
2  
If you do use a temporary variable, write if (!validate) rather than if (validate == false). –  Philip Feb 25 '12 at 18:01
9  
I would name the function something like "CredentialsAreValid()" so you know it should be returning a bool but otherwise yes its good practice –  Zachary K Feb 25 '12 at 18:23
1  
IsValidCredentials, although grammatically awkward, is a common format for indicating a boolean return value. –  zzzzBov Feb 25 '12 at 20:41
    
@Philip what's the difference between if(!validate) and this if(validate) ?? –  KyelJmD Feb 26 '12 at 2:47
    
! is the "NOT" operator, it negates any boolean expression. So if (!validate) is the opposite of if (validate). The if statement will be entered if validate is not true. –  Philip Feb 26 '12 at 13:53

7 Answers 7

up vote 23 down vote accepted

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.

share|improve this answer
3  
+1. Readability is top priority here. –  tdammers Feb 25 '12 at 20:34
    
is that type of readable? like the one I did above –  KyelJmD Feb 26 '12 at 2:47

Why use an additional variable? I prefer use the first approach, it's more readable and simple.

share|improve this answer

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

Yes, if the conditional is not simple enough to inline and have it be readable.

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

You should only do this if your using the value in multiple places or need it to make the code more readable. Otherwise the assignment to a variable is unnecessary. Unnecessary code is at best wasteful and at worst a source of a defect.

share|improve this answer

Let see...

Because it's all about KISS, there's no need to create an additional variable when you can do without it. Also, there's no need to type more... when there's no need.

But then because you DRY, if you were later calling ValidateCredentials and find yourself typing ValidateCredentials(txtUser.Text, txtPassword.Text) you know you should have created an additional variable.

share|improve this answer

Expanding a bit on the readability issue ...

I can think of two good reasons to store the result in a variable:

  1. If you're going to be using the condition more than once, saving it in a variable means you only have to call the function once. (This assumes that the stored value is still valid; if you need to re-test it, of course this doesn't apply.)

  2. If storing it in a variable improves readability by giving the condition a name that's more meaningful than what you see in the function call. For example, this:

    bool foo_is_ok = is_ok(foo); if (foo_is_ok) ...

doesn't help readability, but this:

bool done_processing = feof(file1) && feof(file2);
if (done_processing) ...

probably does, since it's not immediately obvious that feof(file1) && feof(file2) means that we're done processing.

The passwordsMatch variable in the question is probably a better example than mine.

Single-use variables are useful if they give a meaningful name to some value. (This is of course for the benefit of the human reader.)

share|improve this answer

Yes, it is usually fine to use such methods as if conditions. It is helpful though if the method name indicates that the method returns a bool; for instance CanValidateCredentials. In C-style languages this method is often takes the form of Is and Can prefixes, and in Ruby with the '?' suffix.

share|improve this answer
1  
Good point about method names, but I would never start a method name with "if". Then the code reads "if if". "Can" is ok: if (cat.canOpenCanOfCatFood()). –  kevin cline Feb 25 '12 at 22:48
    
Thanks, @Kevin. I meant 'Is' not 'If'. I edited the answer –  Christian Horsdal Feb 26 '12 at 20:11

Good practice is to write code that is correct, readable, efficient.

  1. Correct: mainly it won't make a difference, sometimes transactional or sequential issues will be more clearly described in a single if statement. I.e. a correct solution that is not a single if will be much more complex. Think functions with side effects.
  2. Readability: long names or names that while accurate don't precisely describe you usage, might be usefully replaced with a local variable.
  3. Efficient: calling a function more than once within a single expression, is a slight waste of time if the value can not change between calls. While the imrovement is almost certainly inconsequencial, it also aids readability as it means that either the value can not change between calls or that it can change and you want the initial value.

So, the answer is "it depends", with possibly a slight bias towards using a variable. Note that when using a debugger, holding a value in a variable means that you can hang on to it longer, and not keep having to recalculate it (and of course if it does have side effects then, then it changing everytime your debugger refreshes the value wold be a problem).

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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