Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite
1

I have the following in the header.php file which is included in all of my views:

$dh = opendir(Vs.get_class($this).'/js') ;
while($script = readdir($dh)) {
    if(!is_dir($script))
    {
        echo '<script type="text/javascript" src="js/'.$script.'"></script>' ;
    }
}

$dh = opendir(Vs.get_class($this).'/css') ;
while($css = readdir($dh)) {
    if(!is_dir($css))
    {
        echo '<link type="text/css" href="css/'.$css.'" rel="stylesheet"/>' ;
    }
}

It's purpose is to autoload all the CSS and JS files for a particular view (which has the same name as the controller, hence get_class).

Should all this be a part of the associated controller or is how I have done it fine?

@sudobangbang Thanks I have edited it now so it is:

foreach(glob(Vs.get_class($this).'/css/*.css') as $cssFile) {
    echo '<link type="text/css" href="'.WROOT.$cssFile.'" rel="stylesheet"/>'."\n" ;
}

I do have a base controller class which all my controllers extend from, do you have an example of a helper function you mentioned?

My views file structure is like this:

/views
/views/header.php
/views/footer.php
/views/mypage
/views/mypage/index/php
/views/mypage/css
/views/mypage/js
share|improve this question

1 Answer

up vote 2 down vote

  1. The way you are scanning the directory is going to get all files. If this is intentional thats cool, but if you are really just looking for js & css files, use glob and scan for them directly. This will be much better in performance, and you can avoid the directory check.

  2. It seems like its a brittle idea to have the paths mapped to a specific class name. There has to be an easier way to associate views and view resources rather than referencing $this.

  3. This does not belong in the controller unless you are assigning the view data there. In frameworks like Zend, there are action helpers which take care of this for you. If you don't have access to that, create a base controller class and do a magic helper method to keep the functionality the same across all pages. This will allow you to maintain this functionality closer, and you can still use the $this if needed.

  4. Is this really a good idea to just scan a directory and include all files? What happens if a new developer comes along and dumps a js file they were testing something in, in the directory and it got included by accident? If there are very few includes, try to just manually put them in; either through a layout or just hardcoding them in there. If you really have that much js and css being included in.... maybe there is a more fundamental problem.

Better Scandir w/ Glob:

$jsGlob = Vs.get_class($this).'/js/*.js';
foreach (glob($jsGlob) as $js)
{
    echo '<script type="text/javascript" src="$js"></script>' ;
}

$cssGlob = Vs.get_class($this).'/css/*.css';
foreach (glob($cssGlob) as $css)
{
    echo '<link type="text/css" href="$css" rel="stylesheet"/>' ;
}
share|improve this answer

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.