jleck
jleck

Reputation: 861

PHP switch, why doesn't this work?

I have a strange problem that I can't seem to solve. I've quite a complicated bit of code going on, but I've simplified it and the problem still exists.

See the following:

<?php
$meta = array('meta_title' => 'correct');

switch (true) {
    case empty($meta['meta_description']):
        $meta['meta_description'] = 'incorrect';
    case empty($meta['meta_keywords']):
        $meta['meta_keywords'] = 'incorrect';
    case empty($meta['meta_title']):
        $meta['meta_title'] = 'incorrect';
}

print_r($meta);

Now for some reason, his returns meta_title as incorrect eventhough its clearly set in the array. It's almost as if its ignoring the case and just dropping down.

I've set up an example at: http://codepad.org/mQH9Kf1L

Thanks in advance!

UPDATE

It might make more sense to see where I'm using this. See the following: http://codepad.org/WnxBp8Nt (line 43 onwards)

Just out of interest, I changed I added a quick microtimer and tested this version and a version written with seperate ifs. The if version came out a little slower.

Upvotes: 3

Views: 913

Answers (6)

Francis Avila
Francis Avila

Reputation: 31621

Once a single case is true, all other following code in the switch block is executed except other case statements. See the php documentation for the switch statement:

The switch statement executes line by line (actually, statement by statement). In the beginning, no code is executed. Only when a case statement is found with a value that matches the value of the switch expression does PHP begin to execute the statements. PHP continues to execute the statements until the end of the switch block, or the first time it sees a break statement. If you don't write a break statement at the end of a case's statement list, PHP will go on executing the statements of the following case.

So what happens here is the following:

switch (true) {
    case empty($meta['meta_description']): // MATCH
        $meta['meta_description'] = 'incorrect';  // EXECUTE
    case empty($meta['meta_keywords']): // SKIP
        $meta['meta_keywords'] = 'incorrect'; // EXECUTE
    case empty($meta['meta_title']): // SKIP
        $meta['meta_title'] = 'incorrect'; // EXECUTE
}

Note that the following case statement bodies are not executed at all:

switch(true) {
    case false:
        echo "Not Executed\n";
    case true:
        echo "Executed\n";
    case print("Condition Not Executed\n"):
        echo "Also Executed\n";
}

This will print:

Executed
Also Executed

Upvotes: 1

Chibueze Opata
Chibueze Opata

Reputation: 10054

Quoting from the PHP Documentation for Switch:

It is important to understand how the switch statement is executed in order to avoid mistakes. The switch statement executes line by line (actually, statement by statement). In the beginning, no code is executed. Only when a case statement is found with a value that matches the value of the switch expression does PHP begin to execute the statements. PHP continues to execute the statements until the end of the switch block, or the first time it sees a break statement. If you don't write a break statement at the end of a case's statement list, PHP will go on executing the statements of the following case.

What you are actually trying to do is:

<?php
$meta = array('meta_title' => 'correct');

switch (true) {
    case empty($meta['meta_description']):
        $meta['meta_description'] = 'incorrect';
}
switch (true) {
    case empty($meta['meta_keywords']):
        $meta['meta_keywords'] = 'incorrect';
}
switch (true) {
    case empty($meta['meta_title']):
        $meta['meta_title'] = 'incorrect';
}

print_r($meta);

Upvotes: 2

Cal
Cal

Reputation: 7157

The reason it's not doing what you want is because if case 1 is true, cases 2 & 3 trigger automatically (and if case 2 is true, case 3 always fire). This is not what switch is for. You really just need 3 separate if clauses:

<?php
$meta = array('meta_title' => 'correct');

if (empty($meta['meta_description']))
        $meta['meta_description'] = 'incorrect';
if (empty($meta['meta_keywords']))
        $meta['meta_keywords'] = 'incorrect';
if (empty($meta['meta_title']))
        $meta['meta_title'] = 'incorrect';

print_r($meta);

Upvotes: 4

Travesty3
Travesty3

Reputation: 14479

You don't have any break statements. Add one to each case and it will work.

What's happening is that the first case (empty($meta['meta_description'])) is evaluating to true and without any break statements, the code for the rest of the cases are executed as well, so meta_keywords is set to incorrect and so is meta_title.

Given that this is how switch statements work, you are probably not looking to use a switch statement. You are probably looking for something more like this:

$meta = array('meta_title' => 'correct');

if (empty($meta['meta_description']))
    $meta['meta_description'] = 'incorrect';
if (empty($meta['meta_keywords']))
    $meta['meta_keywords'] = 'incorrect';
if (empty($meta['meta_title']))
    $meta['meta_title'] = 'incorrect';

print_r($meta);

Or you could use a loop:

$meta = array('meta_title' => 'correct');

$properties = array('meta_description', 'meta_keywords', 'meta_title');
foreach ($properties as $property)
{
    if (empty($meta[$property]))
        $meta[$property] = 'incorrect';
}

print_r($meta);

Both of these should produce the same result, but the loop will be easier and shorter if you have more properties.

Upvotes: -1

David B&#233;langer
David B&#233;langer

Reputation: 7438

Break won't solve the problem. If/else won't work since many needs. Also your code make no sense. You ever heard of foreach ?

<?php
$MetaDefault = array('meta_description', 'meta_title', 'meta_keywords');

$Meta = array('meta_title' => 'correct');

foreach($MetaDefault as $Row){
    if(!isset($Meta[$Row])){
        $Meta[$Row] = 'incorrect';
    }
}

print_r($Meta);
?>

If you break, it will quit the switch. Your others variables won't be test. If/else will do the same.

Upvotes: 1

Naftali
Naftali

Reputation: 146310

Remember to break:

$meta = array('meta_title' => 'correct');

switch (true) {
    case empty($meta['meta_description']):
        $meta['meta_description'] = 'incorrect';
        break;
    case empty($meta['meta_keywords']):
        $meta['meta_keywords'] = 'incorrect';
        break;
    case empty($meta['meta_title']):
        $meta['meta_title'] = 'incorrect';
        break;
}

print_r($meta);

Also the above makes no sense.

You should not use a switch statement for the above.

Try using if...elseif...:

if(empty($meta['meta_description']))
    $meta['meta_description'] = 'incorrect';
elseif(empty($meta['meta_keywords']))
    $meta['meta_keywords'] = 'incorrect';
elseif(empty($meta['meta_title']))
    $meta['meta_title'] = 'incorrect';

Upvotes: 1

Related Questions