CODEWITHSUNDEEP
php
DIES
DIES

Reputation: 349

PHP: Can somebody simplify this IF statement for me?

if($object_type == 'regular') {
    if($u_login == $object_user || $u_access >= 3 || $object_access >= 4) {
        echo '<p>&nbsp;</p>';
        echo '<p><span class="r_button"><a href="/delete_link/'.$object_key.'?link_id=2">'.ex_lang('str_btn_delete').'</a></span></p>';
    }
} else
if($object_type == 'comment') {
    if($u_login == $object_user || $u_access >= 2 || $object_access >= 4) {
        echo '<p>&nbsp;</p>';
        echo '<p><span class="r_button"><a href="/delete_link/'.$object_key.'?link_id=2">'.ex_lang('str_btn_delete').'</a></span></p>';
    }
}

So the thing that if object is different type user need to have diff access level. How this statement can be simplified for don't having duplicates in it?

I generally forgot about groups in if's, thank you for reminding me it!

if($u_login == $object_user || $object_access >= 4 || ($object_type == 'regular' && $u_access >= 3) || ($object_type == 'comment' && $u_access >= 2)) {
    echo '<p>&nbsp;</p>';
    echo '<p><span class="r_button"><a href="/delete_link/'.$object_key.'?link_id=2">'.ex_lang('str_btn_delete').'</a></span></p>';
}

Upvotes: 3

Views: 59

Answers (2)

Obsidian Age
Obsidian Age

Reputation: 42374

Both conditions check for $u_login == $object_user and $object_access >= 4, with only the $object_type and $u_access differing. As such, you can bring these two checks up a level, and check against $object_type and $u_access >= 3 inside the outer condition.

As such, the statement can be re-written like this, shrinking one line of code:

if($u_login == $object_user || $object_access >= 4) {
    if($object_type == 'regular' && $u_access >= 3) {
        echo '<p>&nbsp;</p>';
        echo '<p><span class="r_button"><a href="/delete_link/'.$object_key.'?link_id=2">'.ex_lang('str_btn_delete').'</a></span></p>';
    }
    else if($object_type == 'comment' && $u_access >= 2) {
        echo '<p>&nbsp;</p>';
        echo '<p><span class="r_button"><a href="/delete_link/'.$object_key.'?link_id=2">'.ex_lang('str_btn_delete').'</a></span></p>';
    }
}

Although depending on your definition of 'simplify', you could also cut out the outer conditional entirely by making use of some brackets:

if(($u_login == $object_user || $object_access >= 4) && ($object_type == 'regular' && $u_access >= 3)) {
    echo '<p>&nbsp;</p>';
    echo '<p><span class="r_button"><a href="/delete_link/'.$object_key.'?link_id=2">'.ex_lang('str_btn_delete').'</a></span></p>';
}
else if(($u_login == $object_user || $object_access >= 4) && ($object_type == 'comment' && $u_access >= 2)) {
    echo '<p>&nbsp;</p>';
    echo '<p><span class="r_button"><a href="/delete_link/'.$object_key.'?link_id=2">'.ex_lang('str_btn_delete').'</a></span></p>';
}

However, it's worth nothing that both of your conditionals currently do the exact same thing, so the code could even be simplified to:

echo '<p>&nbsp;</p>';
echo '<p><span class="r_button"><a href="/delete_link/'.$object_key.'?link_id=2">'.ex_lang('str_btn_delete').'</a></span></p>';

Hope this helps! :)

Upvotes: 2

B. Fleming
B. Fleming

Reputation: 7230

This probably isn't the best place to ask for help refactoring your code, but what the heck. Notice that you have two conditions that are exactly the same and being checked in both if conditions. Why not pull those up to the root level?

if($u_login == $object_user || $object_access >= 4) {
    if($object_type == 'regular' && $u_access >= 3) {
        echo '<p>&nbsp;</p>';
        echo '<p><span class="r_button"><a href="/delete_link/'.$object_key.'?link_id=2">'.ex_lang('str_btn_delete').'</a></span></p>';
    }

    if($object_type == 'comment' && $u_access >= 2) {
        echo '<p>&nbsp;</p>';
        echo '<p><span class="r_button"><a href="/delete_link/'.$object_key.'?link_id=2">'.ex_lang('str_btn_delete').'</a></span></p>';
    }
}

Notice that we don't need an else anywhere in here because the conditions provided are, by nature, mutually exclusive. This can make readability a bit simpler.

Upvotes: 1

Related Questions