Before I go any further - critique please!

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Re: Before I go any further - critique please!

Post by s.dot »

OK, I've added modifiers to be able to modify display-characteristics variables, such as uppercase and lowercase them.

Here's my current implementation:

Code: Select all

<?php
 
class SMTemplate
{
    private $delimStart = '{ $';
    private $delimEnd = ' }';
    private $modifierChar = '|';
    private $modifiers = array();
    private $vars = array();
    private $templateFilename = '';
    private $templateCode;
    
    public function __construct($templateFilename='')
    {
        if (!empty($templateFilename))
        {
            $this->templateFilename = $templateFilename;
        }
        
        $this->addModifier(array('upper', 'lower'));
    }
    
    public function assign($var, $value=null)
    {
        if (is_array($var))
        {
            foreach ($var AS $k => $v)
            {
                $this->vars[$k] = $v;
            }
        } else
        {
            $this->vars[$var] = $value;
        }
    }
    
    public function setStartDelimeter($startDelim)
    {
        $this->delimStart = $startDelim;
    }
    
    public function setEndDelimeter($endDelim)
    {
        $this->delimEnd = $endDelim;
    }
    
    public function setModifierCharacter($modChar)
    {
        $this->modifierChar = $modChar;
    }
    
    public function clear()
    {
        $args = func_get_args();
        
        foreach ($args AS $arg)
        {
            unset($arg);
        }
    }
    
    private function getTemplate(&$templateFilename)
    {
        return (is_file($templateFilename) && ($this->templateCode = file_get_contents($templateFilename)));
    }
    
    public function addModifier($modifiers)
    {
        if (is_array($modifiers))
        {
            foreach ($modifiers AS $modifier)
            {
                $this->modifiers[] = $modifier;
            }
        } else
        {
            $this->modifiers[] = $modifiers;
        }
    }
    
    private function render()
    {
        $out = $this->templateCode;
        
        if (strpos($out, $this->delimStart) !== false)
        {
            foreach ($this->vars AS $k => $v)
            {
                $out = $this->replace($k, $v, $out);
            }
        }
        
        return $out;
    }
    
    private function modify($modifier, $varValue)
    {
        if (method_exists($this, strtolower($modifier)))
        {
            return call_user_func(array($this, $modifier), $varValue);
        } else
        {
            trigger_error(
                '<strong>SMTemplate:</strong> Unknown variable modifier (' . $modifier . ').',
                E_USER_ERROR
            );
        }
    }
    
    private function lower($varValue)
    {
        return strtolower($varValue);
    }
    
    private function upper($varValue)
    {
        return strtoupper($varValue);
    }
    
    private function replace($varName, $varValue, &$out)
    {
        if (!is_array($varValue))
        {
            if (strpos($out, $varName . $this->modifierChar) !== false)
            {
                foreach ($this->modifiers AS $modifier)
                {
                    if (stripos($out, $varName . $this->modifierChar . $modifier))
                    {
                        $out = str_replace(
                            $this->delimStart . $varName . $this->modifierChar . $modifier . $this->delimEnd,
                            $this->modify($modifier, $varValue),
                            $out
                        );
                        
                        break;
                    }
                }
            } else
            {
                $varToReplace = $this->delimStart . $varName . $this->delimEnd;
            
                if (strpos($out, $varToReplace) !== false)
                {
                    $out = str_replace($varToReplace, $varValue, $out);
                }
            }
        } else
        {
            foreach ($varValue AS $k => $v)
            {
                $out = $this->replace($varName . '.' . $k, $v, $out);
            }
        }
        
        return $out;
    }
    
    public function display($templateFilename='', $return=false)
    {
        if (empty($this->templateFilename))
        {
            $this->templateFilename = $templateFilename;
        }
        
        if (!$this->getTemplate($this->templateFilename))
        {
            trigger_error(
                '<strong>SMTemplate:</strong> Could not load template file (' . $templateFilename . ').',
                E_USER_ERROR
            );
        }
        
        if ($return)
        {
            return $this->render();
        }
        
        echo $this->render();
    }
}
It works! But here's how I'd like it to work. A separate class for dealing with the modifiers needs to be created. The properties of this class would be:

* $modifierChar
* $modifiers

The following methods should be in that class:

* setModifierCharacter()
* addModifier()
* modify()

I feel like the following methods should be in their own separate files and require_once()'d when addModifier() is called

* lower()
* upper()

I know what I need to do (unless you guys have different ideas), but I'm not experienced enough in OO to do it.
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: Before I go any further - critique please!

Post by Christopher »

I think those are commonly called filters. It seems like you could do a more standard implementation.
(#10850)
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Re: Before I go any further - critique please!

Post by s.dot »

Right... right. Well barring the name change from modifier -> filter, I believe I outlined a standard implementation above? :P I just don't know how to integrate the modifier (filter) class in with the main.

Something like this:

Code: Select all

<?php
 
class SMFilter extends SMTemplate
{
    private $filterCharacter;
    private $filters
 
    public function construct()
    {
        $this->addFilter('lower', 'upper');
    }
 
    private function addFilter($filters)
    {
        if (is_array($filters)){ foreach ($filters AS $filter){ $this->filters[] = $filter; } } 
        else { $this->filters[] = $filter; }
    }
 
    private function filter($varValue, $filter, $filterFilename)
    {
        if (file_exists($filterFilename))
        {
             require_once $filterFilename;
        }
 
        return call_user_func($filter, $varValue);
    }
}
Unless you had something different in mind.
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: Before I go any further - critique please!

Post by Christopher »

Why does filter inherit template? Shouldn't the template class just composite filter objects?
(#10850)
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Re: Before I go any further - critique please!

Post by s.dot »

So basically something like...

Code: Select all

class SMTemplate
{
    private $filterObject;
 
    public function __construct()
    {
        $this->filterObject = new SMFilter();
    }
 
    //...........
    public function addFilter($filter)
    {
        $this->filterObject->addFilter($filter);
    }
    //............
}
?

Or do you mean just passing the template through the filtering object internally?

Code: Select all

<?php
 
class SMTemplate()
{
     private function hasFilterChar(&$templateString)
     {
          return (strpos($templateString, $this->filterCharacter) !== false);
      }
 
     private function filter()
     {
          if ($this->hasFilterChar($templateString))
          {
                return SMFilter::filter($templateString);
           }
      }
}
Sorry about all of the questions. My organization of coding is not on par with many seasoned developers. I do like how you answer the questions with a question, though. Makes me think a bit!
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: Before I go any further - critique please!

Post by josh »

scottayy wrote:So basically something like...
yep
scottayy wrote:Or do you mean just passing the template through the filtering object internally?
I don't think so
http://en.wikipedia.org/wiki/Composite_pattern

Cool template system, code looks good
Post Reply