HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Forum Sign up and Sign in software

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
phpcodereviewhtmlstackoverflow
andsignsoftwareforum

Problem

I would love to have your reviews of this sign in and sign up system.

You can see the main structure:

For now, I have a total of 13 files (index.php, top_bar.php, header.php, container.php, footer.php, sign_in.php, sign_up.php, members.php, help.php, contact_us.php, change_theme.php, rules.php and the style.css.

This is how my main page functions:

index.php:


    
        
        
        Test - Forums
        
        
        Test - Members
        
        
        Test - Sign Up
        
        
        Test - Sign In
        
        
        Test - Change Theme
        
        
        Test - Contact Us
        
        
        Test - Help
        
        
        Test - Rules


This is how my main pages (top_bar.php, header.php, container.php and footer.php) are structured:

top_bar.php:


    
        
            
            
            
                Home
            
            
                " href=".">Forums
            
            
                " href="members.php">Members


header.php:


    
        
            Test
        
        
            
                
                    Sign Up
                
                
                    Sign In


container.php:

```









Forums
Categories


Members


Sign Up


Sign In


Change Theme


Contact Us


Help


Rules
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin eu nibh turpis. Nunc sit amet auctor elit. Lorem ipsum dolor sit amet, consectetur adipiscing elit. In malesuad

Solution

There's a few things wrong with this. Primarily that there is no separation of concerns - you have PHP mixed with HTML, etc. However, barring that, there are some low hanging fruit that we can nip:

Your index.php contains unnecessary mixtures of ... etc. With a switch statement, the index.php can become more concise, while retaining the same functionality. Additionally, there's not need for multiple in the includes. Just one will do:


    
        
        Test -


However, seeing as you're not doing any logic on the includes - why not just have it as one page?

For top_bar.php, you can get rid of most of the php code by using this more concise version:


    
        
            
            
            
                Home
            
            
                " href=".">Forums
            
            
                " href="members.php">Members


Again, in container.php as in index.php, a switch statement benefits you. Also, abstracting margin:15px to a CSS class is cleaner:


    .h1_marg_b15{
        margin-bottom: 15px;
    }

    
        
            
                
                
            
        
        Forums';
                echo 'Categories';
            } break;
            case 'members': {
                echo 'Members';
            } break;
            case 'sign_up': {
                echo 'Sign Up';
            } break;
            case 'sign_in': {
                echo 'Sign In';
            } break;
            case 'change_theme': {
                echo 'Change Theme';
            } break;
            case 'contact_us': {
                echo 'Contact Us';
            } break;
            case 'help': {
                echo 'Help';
            } break;
            case 'rules': {
                echo 'Rules';
                echo 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin eu nibh turpis. Nunc sit amet auctor elit. Lorem ipsum dolor sit amet, consectetur adipiscing elit. In malesuada lobortis tempus. Integer auctor condimentum sapien, non scelerisque eros cursus et. In vel leo elementum, iaculis tellus sit amet, vestibulum quam. Etiam dapibus pulvinar risus, vestibulum rhoncus sapien commodo vitae. Etiam sit amet ultrices dui. Suspendisse luctus fringilla eros. Nam vitae metus porttitor, sagittis arcu eleifend, malesuada odio. Aliquam erat volutpat.';
                echo 'Pellentesque id velit a elit porttitor sollicitudin et vulputate nisl. Donec eu purus non libero porta malesuada et non lorem. Vestibulum ultrices vitae elit vitae accumsan. Quisque euismod, quam sed ornare ultrices, magna mi posuere massa, vel placerat ipsum est quis erat. Aliquam non libero mauris. Etiam ligula velit, commodo et feugiat ac, porta eu orci. Donec laoreet ipsum in urna auctor, vitae malesuada nibh consequat. Donec sit amet libero vitae erat rhoncus venenatis. Maecenas nec pretium justo, eget fermentum tellus. Ut aliquet tellus venenatis posuere fermentum. Fusce mattis velit et tellus suscipit consectetur.';
            } break;
        }
        ?>


There are a handful of other things that are of concern (not checking for empty values, etc), but they are inconsequential in this scope.

Putting it all together, into one page, would look something like:

```
Forums';
$sidecase .= 'Categories';
} break;
case 'members': {
$sidecase .= 'Members';
} break;
case 'sign_up': {
$sidecase .= 'Sign Up';
} break;
case 'sign_in': {
$sidecase .= 'Sign In';
} break;
case 'change_theme': {
$sidecase .= 'Change Theme';
} break;
case 'contact_us': {
$sidecase .= 'Contact Us';
} break;
case 'help': {
$sidecase .= 'Help';
} break;
case 'rules': {
$sidecase .= 'Rules';
$sidecase .= 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin eu nibh turpis. Nunc sit amet auctor elit. Lorem ipsum dolor sit amet, consectetur adipiscing elit. In malesuada lobortis tempus. Integer auctor condimentum sapien, non scelerisque eros cursus et. In vel leo elementum, iaculis tellus sit amet, vestibulum quam. Etiam dapibus pulvinar risus, vestibulum rhoncus sapien commodo vitae. Etiam sit amet ultrices dui. Suspendisse luctus fringilla eros. Nam vitae metus porttitor, sagittis arcu eleifend, malesuada odio. Aliquam erat volutpat.';
$sidecase .= 'Pellentesque id velit a elit porttitor sollicitudin et vulputate nisl. Donec eu purus non libero porta malesuada et non lorem. Vestibulum ultrices vitae elit vitae accumsan. Quisque euismod, quam sed ornare ultrices, magna mi posuere massa, vel placerat ipsum est quis erat. Aliquam non libero mauris. Etiam ligula velit, commodo et feugiat ac, porta eu orci. Donec laoreet ipsum in urna auctor, vitae malesuada nibh consequat.

Code Snippets

<!DOCTYPE html>
<html lang="en-us">
    <head>
        <meta charset="UTF-8">
        <title>Test - <?php
        switch(trim($_SERVER['SCRIPT_FILENAME'])){
            case "members":         echo "Members";         break;
            case "sign_up":         echo "Sign Up";         break;
            case "sign_in":         echo "Sign In";         break;
            case "change_theme":    echo "Change Theme";    break;
            case "contact_us":      echo "Contact Us";      break;
            case "help":            echo "Help";            break;
            case "rules":           echo "Rules";           break;
            default:                echo "Forums";          break;
        }?></title>

        <link href="css/style.css" rel="stylesheet" type="text/css">
        <link href="//maxcdn.bootstrapcdn.com/font-awesome/4.1.0/css/font-awesome.min.css" rel="stylesheet" type="text/css">
    </head>
    <body>
        <?php 
          include("top_bar.php");
          include("header.php");
          include("container.php");
          include("footer.php");
        ?>
    </body>
</html>
<!-- TOP BAR -->
<div id="top_bar">
    <div class="wrapper">
        <div id="top_bar_links">
            <ul>
            <?php
            $page_name = array_pop(explode("/", $_SERVER["PHP_SELF"]));
            ?>
            <li id="home">
                <a href="../">Home</a>
            </li>
            <li>
                <a class="<?php echo ($page_name=="index.php")?"active":"";?>" href=".">Forums</a>
            </li>
            <li>
                <a class="<?php echo ($page_name=="members.php")?"active":"";?>" href="members.php">Members</a>
            </li>
            </ul>
        </div>
    </div>
</div>
<style>
    .h1_marg_b15{
        margin-bottom: 15px;
    }
</style>

<!-- CONTAINER -->
<div class="wrapper">
    <div id="container">
        <div id="breadcrumb_top">
            <div class="breadcrumb_links">
                <ul>
                </ul>
            </div>
        </div>
        <?php
        switch(trim($_SERVER['SCRIPT_FILENAME']){
            case 'index': {
                echo '<h1 class="h1_marg_b15">Forums</h1>';
                echo '<h3 id="category_title">Categories</h3>';
            } break;
            case 'members': {
                echo '<h1 class="h1_marg_b15">Members</h1>';
            } break;
            case 'sign_up': {
                echo '<h1 class="h1_marg_b15" style="text-align: center;">Sign Up</h1>';
            } break;
            case 'sign_in': {
                echo '<h1 class="h1_marg_b15" style="text-align: center;">Sign In</h1>';
            } break;
            case 'change_theme': {
                echo '<h1 class="h1_marg_b15">Change Theme</h1>';
            } break;
            case 'contact_us': {
                echo '<h1 class="h1_marg_b15">Contact Us</h1>';
            } break;
            case 'help': {
                echo '<h1 class="h1_marg_b15">Help</h1>';
            } break;
            case 'rules': {
                echo '<h1 style="margin-bottom: 10px;">Rules</h1>';
                echo '<p style="margin-bottom: 15px;">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin eu nibh turpis. Nunc sit amet auctor elit. Lorem ipsum dolor sit amet, consectetur adipiscing elit. In malesuada lobortis tempus. Integer auctor condimentum sapien, non scelerisque eros cursus et. In vel leo elementum, iaculis tellus sit amet, vestibulum quam. Etiam dapibus pulvinar risus, vestibulum rhoncus sapien commodo vitae. Etiam sit amet ultrices dui. Suspendisse luctus fringilla eros. Nam vitae metus porttitor, sagittis arcu eleifend, malesuada odio. Aliquam erat volutpat.</p>';
                echo '<p style="margin-bottom: 15px;">Pellentesque id velit a elit porttitor sollicitudin et vulputate nisl. Donec eu purus non libero porta malesuada et non lorem. Vestibulum ultrices vitae elit vitae accumsan. Quisque euismod, quam sed ornare ultrices, magna mi posuere massa, vel placerat ipsum est quis erat. Aliquam non libero mauris. Etiam ligula velit, commodo et feugiat ac, porta eu orci. Donec laoreet ipsum in urna auctor, vitae malesuada nibh consequat. Donec sit amet libero vitae erat rhoncus venenatis. Maecenas nec pretium justo, eget fermentum tellus. Ut aliquet tellus venenatis posuere fermentum. Fusce mattis velit et tellus suscipit consectetur.</p>';
            } break;
        }
        ?>
        <div id="breadcrumb_bottom">
            <div class="breadcrumb_links">
                <ul>
                </ul>
            </div>
        </div>
    </div>
</div>
<?php
# Select the title
switch(trim($_SERVER['SCRIPT_FILENAME'])){
    case "members":         $title = "Members";         break;
    case "sign_up":         $title = "Sign Up";         break;
    case "sign_in":         $title = "Sign In";         break;
    case "change_theme":    $title = "Change Theme";    break;
    case "contact_us":      $title = "Contact Us";      break;
    case "help":            $title = "Help";            break;
    case "rules":           $title = "Rules";           break;
    default:                $title = "Forums";          break;
}

# Define the page name and check for empty value
$page_name = array_pop(explode("/", $_SERVER["PHP_SELF"])); 
if(empty($page_name)) $page_name = "index.php";

# Select the sidecase
$sidecase = "";
switch(trim($_SERVER['SCRIPT_FILENAME']){
    case 'index': {
        $sidecase .= '<h1 class="h1_marg_b15">Forums</h1>';
        $sidecase .= '<h3 id="category_title">Categories</h3>';
    } break;
    case 'members': {
        $sidecase .= '<h1 class="h1_marg_b15">Members</h1>';
    } break;
    case 'sign_up': {
        $sidecase .= '<h1 class="h1_marg_b15" style="text-align: center;">Sign Up</h1>';
    } break;
    case 'sign_in': {
        $sidecase .= '<h1 class="h1_marg_b15" style="text-align: center;">Sign In</h1>';
    } break;
    case 'change_theme': {
        $sidecase .= '<h1 class="h1_marg_b15">Change Theme</h1>';
    } break;
    case 'contact_us': {
        $sidecase .= '<h1 class="h1_marg_b15">Contact Us</h1>';
    } break;
    case 'help': {
        $sidecase .= '<h1 class="h1_marg_b15">Help</h1>';
    } break;
    case 'rules': {
        $sidecase .= '<h1 style="margin-bottom: 10px;">Rules</h1>';
        $sidecase .= '<p style="margin-bottom: 15px;">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin eu nibh turpis. Nunc sit amet auctor elit. Lorem ipsum dolor sit amet, consectetur adipiscing elit. In malesuada lobortis tempus. Integer auctor condimentum sapien, non scelerisque eros cursus et. In vel leo elementum, iaculis tellus sit amet, vestibulum quam. Etiam dapibus pulvinar risus, vestibulum rhoncus sapien commodo vitae. Etiam sit amet ultrices dui. Suspendisse luctus fringilla eros. Nam vitae metus porttitor, sagittis arcu eleifend, malesuada odio. Aliquam erat volutpat.</p>';
        $sidecase .= '<p style="margin-bottom: 15px;">Pellentesque id velit a elit porttitor sollicitudin et vulputate nisl. Donec eu purus non libero porta malesuada et non lorem. Vestibulum ultrices vitae elit vitae accumsan. Quisque euismod, quam sed ornare ultrices, magna mi posuere massa, vel placerat ipsum est quis erat. Aliquam non libero mauris. Etiam ligula velit, commodo et feugiat ac, porta eu orci. Donec laoreet ipsum in urna auctor, vitae malesuada nibh consequat. Donec sit amet libero vitae erat rhoncus venenatis. Maecenas nec pretium justo, eget fermentum tellus. Ut aliquet tellus venenatis posuere fermentum. Fusce mattis velit et tellus suscipit consectetur.</p>';

Context

StackExchange Code Review Q#58544, answer score: 7

Revisions (0)

No revisions yet.