0

On the top of the file I want to check the user rights, e.g. if logged in, if user has access to the page, what content is visible to them, if the content was published by the user themselves, if all necessary parameters have been passed (e.g. POST-vars) etc.

So, the question was, what would you suggest is the best way (that the code remains readable and understandable, is not too specific for a special page so that it can be implemented on every page, performance as many checks need a database query etc.)

The reason I am asking is, because right now I am doing sth like this:

if($logged_in!=1) {$error="err1";}

else {
    if(...) {...}
    else {...}
}

So, basically that if one check returns acces denied the code won't go through all other ifs but break immediately. I do not use exit or die because I do want to show the page (without content but an error-box), so if I used exit or die it would not display any other code below...

What would you suggest to do?

Thank you very much for your help.

Helmut
  • 1,377
  • 1
  • 15
  • 25

1 Answers1

1

Well, if/else constructs tend to be huge and not really readable. You may consider using Exceptions for failure cases, basically:

<?php
try {

   if($logged_in!=1) {
      throw new Exception("You must be logged in");
   }

   if(!another_condition()) {
      throw new Exception("Another condition failed");
   }

   do_something();
}
catch (Exception $error) {
   display_error_page($error->getMessage());
}

In general, even this is not the most beautiful way: Those situations themselves are a perfect example for the need for object orientation..

See also:

http://php.net/manual/en/language.oop5.php

http://php.net/manual/en/language.exceptions.php

mjhennig
  • 671
  • 5
  • 11
  • I personnally try to avoid using exceptions to check for conditions that can occur from user actions, since those are not "exceptional". I would use try/catch for example to make sure I am connected to the database, or if a remote webservice returns an unexpected error. – SirDarius Jun 03 '12 at 13:42
  • Fair point. But this way you have one generalized mechanism for handling any error case, irregardless whether it's caused by the user of in the processing logic... Why not using some sort of special `UserInteractionException`? One could also call it a `UserError` or `OSILayer8Fatal`... – mjhennig Jun 03 '12 at 13:45
  • thank you mjhennig for your answer and the links and for leading me in the right direction! as I am just learning by doing this finally was a logical example to understand oop! thank you for this! – Helmut Jun 03 '12 at 13:53
  • First of all, performance: http://stackoverflow.com/questions/104329/performance-of-try-catch-in-php checking for exceptions is one thing, but throwing them adds a costly layer of treatment. Then you have to distinguish between "planned" errors, like a user trying to access a forbidden resource, and those exceptional cases where recovery involves breaking from the workflow completely. Just imagine if the preg_match function threw an exception if no match was found and you forgot to catch this exception ? – SirDarius Jun 03 '12 at 13:54
  • @SirDarius: ok, so what would you suggest then? Still go with ifs? – Helmut Jun 03 '12 at 13:56
  • I agree with mjhennig for the last part of his post ! When your code becomes complex, you'll have to find a way to have it better organized, and OOP is a step in that direction. The various MVC frameworks for PHP can provide you many built-in bricks for cases such as yours. – SirDarius Jun 03 '12 at 14:01