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.

I feel that my current business view logic is not efficient or very clean. The problem is building the right output, but with less code and more DRY.

I have 3 'static' links, as in, direct tags:

about, contact, search

I also have three navigation menu's.

The first (URI position 1):

category = ['home', 'cat_1', 'cat_2', 'cat_3', 'cat_4', 'cat_5']

the second (URI position 2):

country = ['all', 'filter_1', 'filter_2', 'filter_3', 'filter_4', 'filter_5']

the third menu (URI position 3), is NOT on ever page, but only on pages that are not category 'home' AND not filter 'all'.

content = ['latest news', 'media and cartoons', 'country profile']

The third menu also does not use its last entry (profile) on any page whose filter is 'all'.

Possible page 'modes' are:

Home Page       // 1 or 2 of the latest articles from all categories
    URI: "/home/all/latest/"

List Page       // list of latest 10 articles using category and filter for WHERE
    URI: "/home/!all/latest/" OR "!home/(any)/latest/"

Article Page    // full article
    URI: "/(any)/(any)/article/"

About, Contact, Search page    // simple pages
    URI: "/about/", "/contact/", "/search/"

I am currently using URI position 1 to load a class of the same name as the URI string; validating if the class exists, and loading home page if it doesn't.

In the class that is instantiated by the URI Position, I have to validate and use if else statements to determine which of the above four page modes to load. As it stands now, that means I have about 9 pages that have a lot of repeating code:

CONTROLLER:

/**  START page
 * = build page elements using templates  */
$page = new TemplateMan();

/**  ADD content
 * = add main page content to output  */
$content_class = str_replace('-', '_', $this->breadcrumbs->getCrumb(1));
try {
    if (!class_exists($content_class)) {
        throw new Exception ('<b>Error - Content class is missing: ' . $content_class . '</b>');
    }
    $content = new $content_class($this->breadcrumbs, $this);
    $content->setMainContent($inc_path);
} catch (Exception $e) {
    echo
        '<p><b>EXCEPTION</b><br />Message: '
        . $e->getMessage()
        . '<br />File: '
        . $e->getFile()
        . '<br />Line: '
        . $e->getLine()
        . '</p>';
}
$page->setDataValues('content', $content->getOutput());

/**  RENDER page
 * = render output to display page  */
$page->buildOutput();
echo $page->renderOutput();

"home" class:

    public function setMainContent($inc_path) {

    /**  ADD main content
     * = add the main content to the output  */
    $array_helper = new ArrayHelp();
    if ($array_helper->recValueReturn($this->breadcrumbs->getCrumb(3), $this->config->getNavThree())) {

        switch ($this->breadcrumbs->getCrumb(3)) {
            case 'latest':
                if ($this->breadcrumbs->getCrumb(2) !== 'all') {
                    $this->output .= '<div>this is the list of articles page</div>';
                } else {
                    $this->output .= '<div>this is the home page</div>';
                }
                break;
            case 'media':
                $this->output .= '<div>this is the media & cartoons page</div>';
                break;
            case 'country':
                if ($this->breadcrumbs->getCrumb(2) !== 'all') {
                    $this->output .= '<div>this is the country profile page</div>';
                }
                break;
            case 'article':
                $this->output .= '<div>this is the article page</div>';
                break;
            default:
                $this->output .= '<div>this is the default (home) page</div>';
                break;
        }
    }
}

How can I make the logic better?

BTW: I have already come up with a different solution that works better since I posted this an hour ago, but I would really like to see what others would come up with.

share|improve this question
1  
You should really take a look at Symfony's Routing, whic as a component, can be used wholly separately from the rest of the project that fall under Sensio. What you had above and what you suggest below are, in my opinion, customization over configuration, when you really should only have to configure it to use it (like a standalone component). –  Jared Farrish Mar 23 '13 at 23:25
    
Thanks! I will look into it. Though I've moved on a fair distance from this in fact.. I've adopted this method: youtube.com/watch?v=Aw28-krO7ZM –  obmon Mar 31 '13 at 11:14

1 Answer 1

The following if..else stack in the controller allows me to load one of the 4 views, while containing the logic in one location, rather than across multiple files.

Still a bit long, but probably the best way:

    /**  ADD content
 * = add main page content to output  */
// array helper
$array_helper = new ArrayHelp();

// Home Page: 1 or 2 of the latest articles from all categories
if ($this->breadcrumbs->getCrumb(3)    == 'latest'
    && $this->breadcrumbs->getCrumb(2) == 'all'
    && $this->breadcrumbs->getCrumb(1) == 'home') {

    //$content = new Homepage();
    $check = '<div><p> Requested page is the <b>Home Page</b></p></div>';
}
// List Page: list of latest 10 articles using category and filter for WHERE
else if (($this->breadcrumbs->getCrumb(3)    == 'latest'
         && $this->breadcrumbs->getCrumb(2) !== 'all'
         && $this->breadcrumbs->getCrumb(1)  == 'home')
         ||
        ($this->breadcrumbs->getCrumb(3)     == 'latest'
         && $array_helper->recValueReturn($this->breadcrumbs->getCrumb(2), $this->getNavTwo())
         && $array_helper->recValueReturn($this->breadcrumbs->getCrumb(1), $this->getNavOne())
         && $this->breadcrumbs->getCrumb(1) !== 'home')) {

    //$content = new ListPage();
    $check = '<div><p> Requested page is a <b>List Page</b> for..<br /> Category: <b>'
        . $array_helper->recValueReturn($this->breadcrumbs->getCrumb(1), $this->getNavOne())[1]
        . '</b> and country filter: <b>'
        . $array_helper->recValueReturn($this->breadcrumbs->getCrumb(2), $this->getNavTwo())[1]
        . '</b></p></div>';
}
// Media & Cartoons Page: list of latest media & cartoons
else if ($this->breadcrumbs->getCrumb(3)       == 'media'
         && $array_helper->recValueReturn($this->breadcrumbs->getCrumb(2), $this->getNavTwo())
         && $array_helper->recValueReturn($this->breadcrumbs->getCrumb(1), $this->getNavOne())) {

    //$content = new MediaPage();
    $check = '<div><p> Requested page is a <b>Media Page</b> for..<br /> Category: <b>'
        . $array_helper->recValueReturn($this->breadcrumbs->getCrumb(1), $this->getNavOne())[1]
        . '</b> and country filter: <b>'
        . $array_helper->recValueReturn($this->breadcrumbs->getCrumb(2), $this->getNavTwo())[1]
        . '</b></p></div>';
}
// Country Profile Page: country information
else if ($this->breadcrumbs->getCrumb(3)    == 'country'
    && $this->breadcrumbs->getCrumb(2)     !== 'all'
    && $array_helper->recValueReturn($this->breadcrumbs->getCrumb(1), $this->getNavOne())) {

    //$content = new ProfilePage();
    $check = '<div><p> Requested page is a <b>Country Profile Page</b> for..<br /> Category: <b>'
        . $array_helper->recValueReturn($this->breadcrumbs->getCrumb(1), $this->getNavOne())[1]
        . '</b> and country filter: <b>'
        . $array_helper->recValueReturn($this->breadcrumbs->getCrumb(2), $this->getNavTwo())[1]
        . '</b></p></div>';
}
// Article Page
else if ($this->breadcrumbs->getCrumb(3) == 'article'
         && $array_helper->recValueReturn($this->breadcrumbs->getCrumb(2), $this->getNavTwo())
         && $array_helper->recValueReturn($this->breadcrumbs->getCrumb(1), $this->getNavOne())
         && is_numeric($this->breadcrumbs->getCrumb(4)) {

    //$content = new ListPage();
    $check = '<div><p> Requested page is an <b>Article</b><br /> Article ID is: <b>'
        . $this->breadcrumbs->getCrumb(4) . '</b></p></div>';
}
// About Page
else if ($this->breadcrumbs->getCrumb(1) == 'about') {

    //$content = new AboutPage();
    $check = '<div><p> Requested page is the <b>About Us Page</b></p></div>';
}
// Contact Page
else if ($this->breadcrumbs->getCrumb(1) == 'contact') {

    //$content = new ContactPage();
    $check = '<div><p> Requested page is the <b>Contact Us Page</b></p></div>';
}
// Search Page
else if ($this->breadcrumbs->getCrumb(1) == 'search') {

    //$content = new SearchPage();
    $check = '<div><p> Requested page is the <b>Advanced Search Page</b></p></div>';
}
else {
    $check = '<div><p> Requested page is either <b>Missing</b> or <b>Incorrect</b>.
    Please use the menus above to navigate the site, or <a href="/home">click here</a> to go to the Home page.</p></div>';
}
share|improve this answer
    
If you have to edit source code to add more tabs (or content) to a web site, you've done something wrong. If you have duplication (such as 'latest') strewn throughout the code, a mistake has been made. –  Dave Jarvis Jan 22 '13 at 23:30
    
can you explain a little further? You talking about the updated method i'm using, or the old? and do you mean 'latest' as in using it multiple times in an if..else stack as above? The new stack only calls a page view once as opposed to the old way i did it.. so not sure what you mean. –  obmon Jan 23 '13 at 4:11
1  
You can map the navigation menu combinations to HTML fragments and encode the rules to choose the correct fragment based on the map. This then requires a single look-up to find the correct check fragment. –  Dave Jarvis Jan 23 '13 at 4:32
    
You're talking about routing, correct? I need to learn that a bit more, didn't fully understand it all the first couple of run throughs, so did this for now. Thanks. –  obmon Jan 23 '13 at 4:36

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.