Site design for upvote/downvote submissions

I don't really have any specific questions for this thread. I have been working on an independent project recently and have been learning everything from the web. Just yesterday I became aware of templating and the idea of separating business and presentational logic.

I would be very thankful for any comments regarding possible re-factorings, security vulnerabilities, improved design philosophies, etc. The purpose of this thread is mostly a sanity check and to be sure my current design and methodology is sound.

index.php

= 1)
        $page = $_GET['page'];
    else
        header('Location: 404.php');
else
    $page = 1;

$startRow = ($page - 1) * $resultsPerPage;

if (isset($_GET['search'])) {
    $searchArgs = explode(' ', $_GET['search']);
    $argCount = count($searchArgs);
    $searchSQL = '\'' . $searchArgs[0] . '\'';

    for ($i = 1; $i  $row, 'upvote' => $upvote, 'votes' => $votes, 'downvote' => $downvote, 'tags' => $tags, 'commentCount' => $commentCount);
}

// Divider

$index_view = new Template('index_view.php', array(
    'header' => new Template('header.php'),
    'menu' => new Template('menu.php'),
    'submissions' => new Template('submissions.php', array('submissions' => $submissions)),
    'pagination' => new Template('pagination.php', array('page' => $page,   'pageCount' => $pageCount, 'resultsPerPage' => $resultsPerPage, 'outcomeCount' => $outcomeCount, 'submissionCount' => $submissionCount, 'sort' => $sort)),
    'footer' => new Template('footer.php')
));

$index_view->render();
?>

submissions.php

    submissions as $row): ?>
        ">
            
                " title="Upvote">
                
                " title="Downvote">
            
            
                .php"> () -  comments
                
                    
                
                 by 
                
            
        
    

index_view.php

```


Website





header->render();

Well, it's good that you thought of it. And you are definitely moving in the right direction. Your code seems to be fine, but still not very readable and not very structured. Try to organize your code to classes or smaller functions. E.g. this:

if (isset($_GET['sort']))
    if ($_GET['sort'] == 'hot' || $_GET['sort'] == 'new' || $_GET['sort'] == 'top')
        $sort = $_GET['sort'];
    else
        header('Location: 404.php');
else
    $sort = 'hot';

switch ($sort) {
    case 'hot':
        $sortAlgorithm = 'submissions.id * (submissions.upvote - submissions.downvote)';
        break;
    case 'new':
        $sortAlgorithm = 'submissions.id';
        break;
    case 'top':
        $sortAlgorithm = '(submissions.upvote - submissions.downvote)';
        break;
}

Should be in function get_sort_algorith(), or even in class Request, or it's subclass. This might seem useless right now, but having code in a function or class makes debugging easier and makes this code re-usable in another parts of the project.

Have you heard/tried any MVC frameworks and/or about MVC design pattern? This pattern describes splitting code to Model (=data & database access), View (=html templates) & Controller (code that manipulates data and calls views for output).

Here are some useful links that might help you:

• Wikipedia's MVC article

• CodeIgniter User Guide chapter on MVC

• and CodeIgniter framework

Some other notes:

header('Location: 404.php');

This probably won't give you what you want. This wold be 301 redirect, not a 404 error. You'd better use header("HTTP/1.0 404 Not Found"); and configure your webserver to map error 404 to 404.php. In apache this could be done with ErrorDocument 404 /404.php

if (isset($_GET['search'])) {
    $searchArgs = explode(' ', $_GET['search']);
    $argCount = count($searchArgs);
    $searchSQL = '\'' . $searchArgs[0] . '\'';

    for ($i = 1; $i < $argCount; $i++) {
        $searchSQL .= ', \'' . $searchArgs[$i] . '\'';
    }

SQL injection here. Assume what will happen if someone will make request with search='; DROP TABLE users. ALWAYS escape what you are getting from outside of your app.

StackExchange Code Review Q#1621, answer score: 7