Bad practice or duct-tape hack?

[xyzzy]

I don't make any attempts to hide my (occasional) disregard for "best practices" or logical clarity in code. I even admitted just how much I do so in a recent thread. In this post I want to present an example of something I did in an application I am writing for a paying client and ask the wiser, more orthodox monks to either explain how (and why) to make my methodology a little more kosher, or perhaps it really is an acceptable WTDI for my purposes.

At the bottom of this explanation is a Catalyst method to load CSS and JS files for my main template. The template wrapper has a section like this:

[% FOREACH file IN css %]<link rel="stylesheet" href='[%c.uri_for("/ro
+ot/static/css/$file.css")%]' />[%END%]
[% FOREACH file IN js %]<script src='[%c.uri_for("/root/static/js/$fil
+e.js")%]'></script>[%END%]

Each action would then push the files that needed to be included into arrayrefs in the stash. This worked at first but was somewhat cumbersome, and required me to remember the names of each file and which were needed for what functionality:

sub register :Local {
  ...
  push @{$c->stash->{js}}, qw/
    jquery-1.7.2.min
    jquery-ui-1.8.20-custom.min
    jquery.form
    jquery.validate
    phone-validate
  /;
  push @{$c->stash->{css}}, qw|
    ui-theme/style
    forms
  |;
}

This quickly became a pain in the ass, but I didn't write a "solution" until this morning. I wanted to reduce the above a single statement like so: $c->forward('include',[qw/jq ui form/]);. I realized that the hash of include definitions should go either into the global YAML configuration file (if I used a YAML config file), or at least with the rest of the application configurations (in the main module for the package), but I don't know enough about how Catalyst works to be able to retrieve this configuration from within my Controller. I also considered that, since this is all template stuff, I should really write this into my config template file (I do have one) and then rewrite my wrapper template logic to pull things fro there, but I really didn't want to figure out how to do that, so I just made an anonymous hash inside the foreach loop that processes the arguments for the include action and passed each argument to it to get the needed files. Actually the anonymous hash is inside the assignment of another hash, which I'm sure adds to the computational complexity, processing time, references floating around behind the scenes, the same thing getting instantiated over and over for no reason, etc. But it makes perfect sense to me.

sub include :Private {
    my ($self, $c, @include) = @_
    foreach (@include) {
        my %links = (
            css => {
                ui => ["aristo/theme"],
                bracket => [qw/ bracket match /],
                datetime => ["datetimepicker"],
                table => ["themes/blue/style"],
                    }->{$_},

            js =>    {
                jq => ["jquery-1.7.2.min"],
                ui => ["jquery-ui-1.8.20.custom.min"],
                form => [qw/jquery.form jquery.validate.min phone-vali
+dation/],
                table => ["jquery.tablesorter.min"],
                datetime => [qw/ jquery-ui-timepicker-addon jquery-ui-
+slider-access/],
                bracket => ["bracket"],
                swap => ['jquery.swap'],
                    }->{$_}
            );
        push @{$c->stash->{css}}, $_ foreach (@{$links{css}});
        push @{$c->stash->{js}}, $_ foreach (@{$links{js}});
    }
}

[sundialsvc4]

I suppose the only comment I could make would be that you are pushing a lot of this HTML-specific logic rather far away from “the presentation layer” of the program.   If I needed to change the CSS and/or JS library requirements, it would be difficult to do so.

I would first weigh the thought that maybe it doesn’t really matter if the CSS and/or JS lists change from screen to screen given that the stuff will probably be compiled-once and then cached anyway.

As my next thought-strategy, I would consider whether there are not, in fact, “just a few general cases.” If so, I could prepare templates containing the various possible combinations of CSS/Script tags in actual use, and then [%include%] those templates into other templates as needed.

I can, after all, have as many templates as I wish, and I can include common segments (be they CSS/Script declarations or something “visible”) as I please.   All of these strategies would serve to yank those lists of CSS/Script dependencies out of the Perl code, where I do not advise they should be, and puts them into the template system where I suggest that they properly belong.   The page-generating code, using a defined constant rather than a literal string, would specify which wrapper-template should be used.   The common page-generating subroutine would contain the actual logic, once, for invoking the templating system properly, and it would be a common choke-point for detecting errors anywhere in the calling code.

Am I going to “look you up and beat you over the head with a wet noodle” if you don’t do it this way?   No, but I do think that this include-based strategy might play out much cleaner and prove to be much easier to maintain.

[tobyink]

Hmmm... I was doing something pretty similar just this evening. Though my hard-coded values for those attributes will eventually be replaced by a config file of some sort.

[sundialsvc4]

I didn’t read your posting closely, I must admit ... but in general my thought on this matter is that config-files and so forth are probably not required.   There probably are only a small handful of actual differences that are actually required.   And this just might be the perfect case for “wrappers.”   In general, my prevailing thought is that Perl code should not have to embody any sort of “knowledge” about what the exact HTML sequence to be issued may require.   It simply ought to be free to point to one-of-a-handful of “canned” use-cases, without having to be very specific about any of them.   The templating structure does the rest.   Thus, if any changes in the future are necessary (as, inevitably, they will be...) the Perl code does not need to be changed in the slightest.   The Perl code only has to designate, “Door #1, Door #2, or Door #3.”   It does not have to bother itself with exactly what is behind them, so, as the contents of those Doors inevitably changes, the Perl code does not require any change whatsoever.

(Hey, if a measly change in the file-name of the JQuery library, say, obliged me to make terrifying changes to a bunch of Perl modules, heck, I just might go after you with a wet noodle!)