I think Drupal should avoid using exit calls and let every function ends naturally

drupal_goto and cron calls implies exit calls which aren't clear at all

resulting in bad code style
for instead

function xyz() {
  if (...) {
    drupal_goto(...);
    // this point won't be reached due to drupal_goto calling exit
  }
  ...
  return ...;
}

Comments

gpk’s picture

gpk commented

In the case you cite, http://api.drupal.org/api/function/drupal_goto/7, we are doing a redirect and so normally there would be no chance for the function to end naturally...

arhak’s picture

arhak commented

currently D7 has more clean exit calls

as you can see the point where a redirection is decided is not enough argument to abruptly terminate the execution e.g. drupal_goto gives a chance to drupal_exit to invoke hook_exit, nevertheless afterwards it calls exit, which I think should be avoided.

as I see it, the only reason to abruptly terminate an execution would be a fatal AND non-recoverable error

even if a fatal error is detected it might be possible to render some output safely without an unnatural termination, but some cases might compromise the stability of the system in which cases an abrupt halt might be appropriated

in any other case, the execution flow should be respected/honored like the original post example illustrates: a disjunctive casuistic path which ends alternatively through return or drupal_goto which is totally incorrect from a readable code perspective

arhak’s picture

arhak commented

a live example of what I think is bad practice #720868: drastic drupal_not_found & exit in argument_handler::query

why should a block being rendered invoke exit in the middle of a function named "query" with a comment stating the purpose of "See if we're outside the allowed date range for our argument" ??

this is being encourage by core's code abusing drupal_not_found & exit

if the final outcome of a page dispatch is gonna be a redirect/not found/exit/whatever it might be done letting functions end naturally and executing drastic actions at the end of the entry point (index.php or menu_execute_active_handler, etc) and shouldn't be based on the result being numeric either, it should be something more robust

arhak’s picture

arhak commented

another example to illustrate why killing PHP might be a bad idea even when it seems there is nothing else to do #286263: Make search indexing smart to handle nodes wth php redirects

kars-t’s picture

kars-t commented
Status: Active » Fixed

Hi

I am closing this issue to clean up the issue queue. Feel free to reopen the issue if there is new information and the problem still resides. If not please make sure you close your issues that you don't need any more.

Maybe you can get support from the local user group. Please take a look at this list at groups.drupal.org.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Angry Dan’s picture

Angry Dan commented

Another reason why this is bad is if you come to unit test your code. Tests won't complete because the code is exiting. Even if you can justify leaving things as they are for now, drupal_exit should be deprecated in favour of natural code execution.