Standardized returning values - is it a good or bad idea

Question

I'm working in PHP (but in this case I think the programming language doesn't matter), and in my class methods I usually meet the following situations:

1. Method has to return true or false
2. Method has to return true or error message
3. Method has to return true + success message or false + error message
4. Method has to return true + success results (object, array, whatever) or false
5. Method has to return true + success results (object, array, whatever) or false + error message
6. etc.

My problem is, that when I use this class methods in my code somewhere I always have to come back to the class, and check what is the method actually returning: simply true or false, true or error message, etc.

Is a good idea to standardize the returning values? If yes, how?

My idea is:

1. if function has to return true or false then simply return true or false

2. if function has to return true or error message then:

   if (success)
   {
       return array(
           TRUE,
           null
       );
   }
   else
   {
       return array(
           FALSE,
           $error_message
       );      
   }
   

3. if function has to return true + success message or error message then:

   if (success)
   {
       return array(
           TRUE,
           $success_message,
       );
   }
   else
   {
       return array(
           FALSE,
           $error_message
       );      
   }
   

4. etc.

I hope you understand guys my problem, even thought my explanation wasn't so good :) What are your suggestions, or best practices? How should I handle this?

UPDATE: Let's take a simple example:

function login($username, $password) 
{
    // Login logic here ..
    if ($logged_in) 
    {
        return TRUE;
    }
    {
        return $error_message;
    }
}

So the correct way to do this will be: return true, or throw exception, and when calling the login method do in withing a try catch. So, when somethong goes wrong (validation fails, etc) I should use exceptions.

Answer 1

I'd say the premise of returning a boolean and something else is misguided.

A function should have a clear purpose with a clear result. If this result can be achieved, the result is returned. If the result cannot be achieved, the function either returns false or throws an exception. Which is better depends on the situation and your general error-handling philosophy. Either way, it's not typically useful to have a function return an error message. That message is not useful to the code that called the function.

PHP has its own mechanism to output error messages in addition to returning false results: trigger_error. It's purely a tool to aid debugging though, it doesn't replace the standard return value. It can be a good fit for cases where you want to display error messages purely to aid the developer though.

If a function is complex enough to possibly result in several different types of errors that need to be handled differently, you should use exceptions to do so.

For example, a very simple function with a clear purpose that only needs to return true or false:

function isUserLoggedIn() {
    return $this->user == 'logged in';
}

A function with a purpose that may fail to fulfill that purpose:

function convertToFoo($bar) {
    if (!is_int($bar)) {
        return false;
    }
    // here be dragons
    return $foo;
}

The same function that also triggers a message, useful for debugging:

function convertToFoo($bar) {
    if (!is_int($bar)) {
        trigger_error('$bar must be an int', E_USER_WARNING);
        return false;
    }
    // here be dragons
    return $foo;
}

A function that may legitimately run into several different kinds of errors that the calling code needs to know about:

function httpRequest($url) {
    ...

    if (/* could not connect */) {
        throw new CouldNotConnectException('Response code: ' . $code);
    }

    ...

    if (/* 404 */) {
        throw new PageNotFoundException('Page not found for ' . $url);
    }

    return true;
}

---

And I'll paste this comment here as well:

It should not be the responsibility of the function to prepare, return or display an end-user error message. If the purpose of the function is to, say, fetch something from the database, then displaying error messages is none of its business. The code that called the fetch-from-database function merely needs to be informed of the result; from here there needs to be code whose sole job it is to display an error message in case the database function cannot get the required information. Don't mix those two responsibilities.

Answer 2

In specific cases, your solutions to return arrays with more than one element which is NOT a collection of entities of the same kind may be an acceptable solution. But in general it's a code smell.

Some problems such a design can hint at:

• You are not using exception handling properly
• Your design goes against the single responsibility principle
• You're always waiting with the return until the end of the method

If a method really fails to do what it is supposed to do, it should not return anything, it should throw an exception.

If your method does more than exactly one task, you have to refactor.

Return as early as possible. Write methods more like this:

if (!$something)
{
    return FALSE;
}

//do some other stuff
return 'great, it worked';

Your specific login function should not return a message. Such action specific user messages should be decoupled and handled by a message queue.

So you could have a Messenger class which is injected into your controller so you can use it anywhere and add messages to the queue.

function login($username, $password) 
{
    // Login logic here ..
    if ($logged_in) 
    {
        $this->messenger->addMessage('success', 'You are loggend in.');
        return TRUE;
    }
    {
        $this->messenger->addMessage('error', $message);
        return FALSE;
    }
}