current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I have a Drupal site with roughly the following in my template.php hook_init block:

function mytemplate_init() {
    if (request_path() == "widgets_json") {
        drupal_json_output(_get_widgets());
        exit;
    }
    if (request_path() == "people_json") {
        drupal_json_output(_get_people());
        exit;
    }
}

Are there any problems with this approach?

On the face of it, the two main problems are that it's going to break if URL's ever change, and it's impossible to theme the output of those functions using an appropriate page--widget_json.tpl.php or page--people_json.tpl.php template. On the other hand, adding a content type just for a few JSON output pages that are never going to change paths seems like overkill.

What do you think of this approach?

share|improve this question

1 Answer 1

up vote 0 down vote

From what I see, you could use two different methods: Ternaries and switches:

From a switch point of view, you could use the following to proceed:

function mytemplate_init() {
    switch(request_path()) {
       case "widgets_json":
           drupal_json_output(_get_widgets());
       break;
       case "people_json":
           drupal_json_output(_get_people());
       break;
    }
}

Or, one can replace that with a ternary to shorten it:

if (request_path() == "widgets_json") {
    drupal_json_output(_get_widgets());
    exit;
}
if (request_path() == "people_json") {
    drupal_json_output(_get_people());
    exit;
}

into:

request_path() == "widgets_json" ? drupal_json_output(_get_widgets()) : (request_path() == "people_json" ? drupal_json_output(_get_people()) : "" )
exit;

The shortside to ternaries is that you sacrifice readability for effectiveness.

However, if you can move past that, ternaries are an effective part of a Programmer's Toolbelt(TM)

Or, as I've since come to understand, we can use objects!

$functions = [
       "widgets_json" => _get_widgets,
       "people_json"  => _get_people
     ];
$functions[request_path()]();

Unfortunately, with such a small amount of code supplied, very little can be reviewed.

share|improve this answer
    
"shorter" doesn't always mean "simpler". In this case, I think the original is simpler: you know what's going on with only half eye open. The ternary is very very far from that. –  janos♦ Jun 27 at 16:17
    
Yes, you're very right. –  Quill Jun 27 at 16:25
1  
Also, readability has been sacrificed as a trade-off. –  Kid Diamond Jun 27 at 17:18
    
Pretty old post, surprised to see it get an answer - I probably should have specified that I wanted answers from a Drupal perspective, but I thought the "Drupal" tag and the "on the face of it" para should make it fairly clear what my concerns are? I'm aware that I could rewrite the code with a switch or a ternary, but tbh I think both of those options are less clear than the current code. –  George Jun 30 at 5:23
    
I see, well, that's unfortunate. –  Quill Jun 30 at 5:26

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.