Is using PHP if/else within JS bad practice?

Question

I'm building a custom template for WordPress and in a couple places I've used PHP if else statements like the following example within the JS in the footer. It works fine but I'm wondering if this is considered "bad practice" and if so what is a better way to handle it?

<script type="text/javascript">
var $submenu = $('.submenu');
// SUBMENU animation
<?php if ( in_category('Collection') == false ) { ?> // if not a Collection subpage
    $('.menu li a').each(function() { 
        if ( $(this).text() == 'Collection' ) { // If is Collection link show submenu on hover
            $(this).mouseenter(function() { 
                $submenu.slideDown(); 
            });
        } else { // else close submenu on hover over other menu links
            $(this).mouseenter(function() { 
                $submenu.slideUp(); 
            });
        }
    });
    $('.nav').mouseleave(function() { // close submenu
        $submenu.slideUp(); 
    });
<?php } else { ?> // If a Collection subpage always show subnav
    $submenu.show();
<?php } ?>
</script>

Not at all. This way you end up with clean code with concerns properly separated. The alternative would be that you would have a javascript variable var inCategory = <?= in_category("Collection") ?>. And then you can check the inCategory variable by javascript. However the way you did it is ok. Only thing you could do is clean up the if statement and use <?= if (in_category("Collection")) : ?> and then <?= else: ?> and in the end <?= endif ?> — Sterling Duchess
– Sterling Duchess, Commented Apr 3, 2013 at 22:11
An alternate approach would be to simply set a variable in the page via PHP, e.g. var isCollection = <?php echo in_category('Collection') ? 1 : 0 ?>. Your JS could then check this var and do the appropriate thing. One advantage is you could then move your JS into an external file, which can be cached, rather than re-sending the same JS payload embedded in every page. — Frank Farmer
– Frank Farmer, Commented Apr 3, 2013 at 22:13
makes maintaining code more difficult mixing server code and javascript. — charlietfl
– charlietfl, Commented Apr 3, 2013 at 22:20
i consider this, and mixing html and js, as bad practice. because you could not automate code validation with tools like jshint or jslint for your scripts. — t.niese
– t.niese, Commented Apr 3, 2013 at 22:22
Lest someone wonders, the upvotes mean this is a good question not a good approach. — Majid Fouladpour
– Majid Fouladpour, Commented Apr 3, 2013 at 22:29

Pebbl · Accepted Answer · 2013-04-03 23:01:02Z

2

Whilst there isn't anything really wrong with mixing PHP and JavaScript, I personally find it quite awkward to read and modify, plus it makes moving that code around tricky. For example if you decided to export that JavaScript to an external file, which has numerous benefits:

<script src="myjs.js.php"></script>

This becomes clunky if your JavaScript needs to know certain values in order to calculate in_category('Collection') as you have to start using GET parameters (unless you are depending on session variables, which can get quite compex and unpredictable, especially through asset requests):

<script src="myjs.js.php?random_vars_required=123"></script>

Another point to be wary of is when having a JavaScript file that changes it's content depending on server-side logic, you have to be careful with what the browser is caching (to avoid these type of problems, it basically means you have to change the request URL for each possible outcome of the js file). i.e.

<script src="myjs.js.php?collection=true"></script>
<script src="myjs.js.php?collection=false"></script>

Another downside is by mixing PHP with JS you are likely to end up duplicating the PHP code in numerous places which goes against the DRY principal. This is why the suggested "export data to a javascript variable" is a much nicer idea. However it's best to avoid variables in the global js namespace if possible. Avoiding the global namespace can prove tricky though if you need to share the logic across multiple JavaScript files and don't wish to export your variables at the top of every file.

another possibility

If the logic you are testing is purely boolean in nature, and it also centres around page classification (or sub-region classification), the following is quite a nice way to handle what you are trying to achieve. It's nice mainly because it keeps your PHP and HTML together, and your JS separate.

The following should be placed in whatever template you use to generate your outer HTML:

<?php

    $classes = array();
    if ( in_category('Collection') ) {
      $classes[] = 'collection';
    }
    $classes = implode(' ', $classes);

?>
<!-- 
  obviously you'd render the rest of the html markup
  I've removed it for simplicity
//-->
<body class="<?php echo $classes; ?>"></body>

Then in your JavaScript / jQuery:

if ( $('body.collection').length ) {
  /// if collection sub page
}
else {
  /// else do otherwise
}

If you'd rather not add a class to your body element, you could always define your boolean check based on something that already exists on one version of the page and not on the other. Although personally I like to keep things clean and only resort to those kind of checks when I know the HTML markup is not going to change much in the future.

Nearly all browsers that the greater world should be worrying about today support multiple classes on elements. So this means even if you have multiple things you wish to check for, as long as it makes sense, you can place these classes on your html or body tag and use jQuery's Sizzle implementation to find them for you.

edited Apr 3, 2013 at 23:01

answered Apr 3, 2013 at 22:37

Pebbl

36.1k6 gold badges66 silver badges65 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Beetroot-Beetroot Over a year ago

Pebbl, via completely different routes, I think we ended up in almost exactly the same place! And I enjoyed reading your answer.

Pebbl Over a year ago

@Beetroot-Beetroot Heh thanks, Yep.. that's because it's a logical way to do it ;) nice work yourself.

Pebbl Over a year ago

Anyone care to comment on the downvote? Seems a bit random... especially if there is no explanation as to the point of view involved?

Beetroot-Beetroot Over a year ago

Anon downvoting has to be the worst feature of SO. T'was a +1 from me BTW.

Pebbl Over a year ago

@Beetroot-Beetroot I'd have to agree. With a down vote the system should force the user to make a comment. Anyways cheers for the +1, you got one from me also :)

Beetroot-Beetroot · Accepted Answer · 2013-04-03 23:00:37Z

Building javascript server-side is probably something we've all done, despite the main arguments for not doing so - namely that the js can't be (easily) validated (with eg. jsLint), and can't (easily) be put into a .js file - there's no point allowing the browser to cache just one of two or more possible versions of the script.

You could consider trading off server-side branching for client-side branching, which arguably makes the code more readable but, more importantly, is an intermediate step to my final suggestion (bear with me) :

var $submenu = $('.submenu');
// SUBMENU animation
var isCollection = <?php echo in_category('Collection') ? 'false' : 'true' ?>;
if ( !isCollection ) { // if not a Collection subpage
    $('.menu li a').each(function() { 
        if ( $(this).text() == 'Collection' ) { // If is Collection link show submenu on hover
            $(this).mouseenter(function() {
                $submenu.slideDown(); 
            });
        } else { // else close submenu on hover over other menu links
            $(this).mouseenter(function() {
                $submenu.slideUp(); 
            });
        }
    });
    $('.nav').mouseleave(function() { // close submenu
        $submenu.slideUp();
    });
} else { // If a Collection subpage always show subnav
    $submenu.show();
}

However, if the boolean isCollection could be determined by another means (eg. by enquiring some aspect of the DOM such as a data-xxx attribute), then you're cooking with gas. Only one version of the js script would be necessary; it could be easily validated with jsLint; and could be moved into a .js file if desired.

Of course you need to set the data-xxx attribute (or whatever) elsewhere in the server-side code (complete with an explanatory comment), which is a possible downside, but maybe not a big one.

Maybe not all js would be amenable to this approach but I think the example in the question would be.

To my mind, this is a viable way ahead on this occasion.

jgillich · Accepted Answer · 2013-04-03 22:34:48Z

1

At least its not a sign of great code. There are alternatives:

Generate a JSON object and parse it in JavaScript
Dynamic inclusion of JS files

Just set conditions:

if(<?= (int)$mybool ?>) {
    doSomething();
}

answered Apr 3, 2013 at 22:34

jgillich

77.2k7 gold badges61 silver badges89 bronze badges

Collectives™ on Stack Overflow

Is using PHP if/else within JS bad practice?

3 Answers 3

another possibility

5 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

another possibility

5 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related