Is it wrong to use the return statement this way?

Question

I've got this autoloader method which is used to include class files as needed.

public static function autoloader($className) {

    $fileExists = false;
    foreach(array(LIBS_PATH, CONTROLLERS_PATH, MODELS_PATH) as $path) {

        // Check if the class file exists and is readable
        if(is_readable($classPath = $path . '/' . $className . '.php')) {
            // Include the file
            require($classPath);
            $fileExists = true;
            break;
        }
    }

    if($fileExists === false) {
        exit('The application cannot continue as it is not able to locate a required class.');
    }
}

That works fine but I was thinking is this way better:

public static function autoloader($className) {

    foreach(array(LIBS_PATH, CONTROLLERS_PATH, MODELS_PATH) as $path) {

        // Check if the class file exists and is readable
        if(is_readable($classPath = $path . '/' . $className . '.php')) {
            // Include the file
            require($classPath);
            return;
        }
    }
    exit('The application cannot continue as it is not able to locate a required class.');
}

As you can see I'm using the return statement in the middle of the loop to terminate the rest of the function because the class file has been included and the method has done its job.

Which way is the best way to break out of the loop if a match has been found? I'm just unsure about using the return statement because I always associated that with returning some value from a method.

The question is subjective and is likely to invite yes/no answers. — samayo, Dec 12 '12 at 16:33
It's mostly a style issue. Some people will argue for having only a single `return` statement at the end of the function. I say go for whatever makes the code more comprehensible. — deceze, Dec 12 '12 at 16:33
@Eritrea Yes, it is subjective, but that doesn't mean an answer saying "no" is incorrect. When things are *subjective*, they are thus because it's about opinion rather than technical facts, so "it's not wrong" is correct and a "yes/no" answer at the same time, *without* being subjective. — phant0m, Dec 12 '12 at 16:40

score 3 · Accepted Answer · answered Dec 12 '12 at 16:34

Return can be used to simply break out of a method/function, and this use is perfectly legal. The real question to ask is what impact do you feel it has on readability?

There are different schools of thought on early returns.

One school maintains a single entry/exit point, stating that it makes the code easier to read since one can be assured that logic at the bottom of the code will be reached.

Another school states that the first school is outdated, and this is not as pressing a concern, especially if one maintains shorter method/function lengths.

A medium exists between the two where a trade-off between an early return and convoluted logic is present.

It comes down to your judgment.

Ok thanks I think I'll do it the second way because it looks cleaner and easier to read to me. — David, Dec 12 '12 at 16:41

phant0m · Answer 2 · 2012-12-12T16:56:13.693

1

No, it is not wrong to use return this way.

edited Dec 12 '12 at 16:56

answered Dec 12 '12 at 16:34

phant0m

16,595
5
50
82

Perhaps you could suggest a better way? – Konrad Viltersten Dec 12 '12 at 16:52
@KonradViltersten Why? I'm not saying it's bad, I'm saying it's not. Being "bad" in this case is a matter of opinion, there's no technically correct way to do it, so all there is to say is "It's not wrong" ;) – phant0m Dec 12 '12 at 16:55
I'm so sorry. My eyes read something entirely different. My bad... I guess I'm too tired after 10 hours at work. – Konrad Viltersten Dec 12 '12 at 17:02
@KonradViltersten No worries, I realized that this might have been the case afterwards, which is why I put it in bold ;) – phant0m Dec 12 '12 at 17:03
1

Exactly. For some reason, the eyes are trained to see *wrong* and jump to that word like if it's flashing red. Nevertheless, one's supposed to be able to read before passing judgements on contributions of others, which I failed to do. Please accept my apologies. I blame Apple and iPhone. – Konrad Viltersten Dec 12 '12 at 17:05

score 0 · Answer 3 · answered Dec 12 '12 at 16:35

0

At least look at manual:

http://php.net/manual/en/function.autoload.php

void __autoload ( string $class )

void means that it should not return anything.
But it is not an error.

And also better use require_once when including class definitions.

answered Dec 12 '12 at 16:35

Bogdan Burym

5,482
2
27
46

1

He's not returning anything ;) Well, he is, but that is `void` which is "nothing".. which it's not... – phant0m Dec 12 '12 at 16:35
Im not returning anything and I don't use the __autoload method I use my own method and register it with spl_autoload_register(). – David Dec 12 '12 at 16:37
To `return` and *to `return` something* are two different things. – deceze Dec 12 '12 at 16:37
I thought using require_once can be very bad and slow. I think using the spl_autoload_register() PHP keeps track of what classes have been included already so there should be no problem using just require as far as I know. – David Dec 12 '12 at 16:39
But imagine that those file was included anywhere before, not by `autoloader`. That is only my thought. I just share the way I would do it. – Bogdan Burym Dec 12 '12 at 16:40

Is it wrong to use the return statement this way?

3 Answers3