Reorganizing unorthodox PHP phone book project

Question

I've been learning PHP and I've created a phone book webpage as an exercise. I am satisfied with the way it works -- it seems to handle all possible input cases correctly, and the interface feels nice and solid. I would appreciate feedback on the code style, as well as suggestions for what would be a better or more idiomatic way to organize it.

<html>

<head>

<title>Phone Book</title>

<link rel="stylesheet"
    href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css">

<link rel="stylesheet"
    href="//fonts.googleapis.com/css?family=Ubuntu+Mono:300,400,600,700&amp;lang=en" />
<style>
body {
    font-family: Ubuntu Mono;
    background-color: #F2F1F0;
}

.form-control {
    border-radius: 0;
    border: 1px solid #AEA79F;
}

.btn-default {
    background-color: #EEEDEB;
}

.form-control:focus {
    border: 1px solid #E95420 !important;
    box-shadow: none !important;
}

.btn:focus, .btn:active {
    outline: none !important;
    box-shadow: none;
}

.header {
    text-shadow: 0px 1px #666;
    box-shadow: 0 0px 0 #F6BBA6 inset;
    color: #F6F6F5;
    background-color: #E95420;
    font-size: 28px;
    font-weight: bold;
    padding: 1px;
    padding-left: 20px;
    border: 1px solid #555;
    margin-top: 12px;
}

.c2 {
    background-color: #FFF;
    border: 1px solid #666;
    border-top: none;
    padding: 18px;
}

.msg {
    border: 1px solid #AEA79F;
    border-style: dashed;
    padding: 8px;
    margin-bottom: 18px;
}

.table td, th {
    border: 1px solid #AEA79F !important;
    border-style: dotted !important;
}

.spc {
    padding: 18px;
}

.inv {
    display: none;
}

input[type="radio"] {
    margin: 5px;
    margin-top: 4px;
    vertical-align: middle;
}

input:invalid {
    border: 1px solid red;
}
</style>

</head>


<body>

    <div class="container">

        <div class="header">Phone Book</div>

        <div class="c2">

            <form action="PhoneForm.php" method="post">

                <div class="col-xs-5" style="padding: 1px">
                    <input type="text" class="form-control" name="name" id="name"
                        placeholder="Name" maxlength="36">

                </div>
                <div class="col-xs-5" style="padding: 1px;">

                    <input type="text" class="form-control" name="phone" id="phone"
                        placeholder="Phone Number" maxlength="17">

                    <div class="phonetypes" class="inv">
                        <input type="radio" name="phonetype" value="other" CHECKED>Other<input
                            type="radio" name="phonetype" value="home">Home <input
                            type="radio" name="phonetype" value="work">Work <input
                            type="radio" name="phonetype" value="cell">Cell <input
                            type="radio" name="phonetype" value="fax">Fax
                    </div>

                </div>
                <div class="col-xs-1" style="padding: 1px">
                    <select class="form-control" id="actions" name="actions">
                        <option value="add">Add</option>
                        <option value="find" selected>Find</option>
                        <option value="delete">Delete</option>
                        <option value="update">Update</option>
                        <option value="viewall">View All</option>;
                    </select>
                </div>

                <div class="col-xs-1" style="padding: 1px">
                    <input type="submit" class="btn btn-default form-control"
                        name="btn" value="Go">
                </div>
                <BR>

                <div class="spc"></div>

            </form>

            <script>
            
            var actions = document.getElementById('actions');
            var phonetypes = document.querySelector('.phonetypes');
            var phone = document.getElementById('phone');
            var name = document.getElementById('name');

            //var pattern_phone = phone.getAttribute("pattern");
            //var pattern_name = name.getAttribute("pattern");

            phonetypes.classList.add('inv');
            actions.addEventListener('change', function() {

                if (actions.value == 'update') {
                    name.placeholder = "Old Name";
                    phone.placeholder = "New Name";
                   // phone.setAttribute("pattern", pattern_name);
                } else {
                    name.placeholder = "Name";
                    phone.placeholder = "Phone Number";
                    //phone.setAttribute("pattern", pattern_phone);
            }

            if (actions.value == 'add') {
                phonetypes.classList.remove('inv');
            } else {
                phonetypes.classList.add('inv');
            }
            });
        </script>

<?php
$host = "localhost";
$user = "user";
$pass = "pass";
$db = "db";

$mysqli = new mysqli($host, $user, $pass, $db);

if (mysqli_connect_errno()) {
    die("Unable to connect.");
}

$show_all = "select Name, PhoneNumber, PhoneType, person.PersonID from person,phone where person.PersonID = phone.personID";
$listquery = $show_all;
msg_info("Displaying all entries.");

$name = trim($_POST["name"]);
$phone = trim($_POST["phone"]);
$phonetype = trim($_POST["phonetype"]);
$action = trim($_POST['actions']);

if ($btn = $_POST['btn']) {

    switch ($action) {

        case ('viewall'):

            $listquery = $show_all;
            msg_info("Displaying all entries.");
            break;

        /* --------------------------------------------------------------- */

        case ('add'):

            if (! ($name or $phone)) {
                msg_warn("Enter a name and/or number to add.");
                break;
            }

            if (! ($name)) {
                msg_warn("Enter a name to be associated with this number.");
                break;
            }

            // is person already there?
            $query = "select PersonID from person where Name like ('" . $name .
                    "')";
            if ($id = $mysqli->query($query)->fetch_object()->PersonID) {

                // if person is there and we have an input phone number, assign
                // the number to him.
                if ($phone) {

                    // check to see if he already has this number
                    if ($result = $mysqli->query(
                            "select * from phone where PhoneNumber like '" .
                            $phone . "' and PersonID = " . $id . "")->fetch_object()) {
                        msg_warn(
                                "Person '" . $name . "' already has number '" .
                                $phone . "'.");
                        break;
                    }

                    // add new phone
                    $q1 = "insert into phone (PersonID, PhoneNumber, PhoneType) values (" .
                            $id . ", '" . $phone . "', '" . $phonetype . "')";

                    // get rid of dummy phone if there is one
                    $q2 = "delete from phone where PersonID = " . $id .
                            " and PhoneNumber = 'none'";

                    // queries okay?
                    if ($mysqli->query($q1) && $mysqli->query($q2)) {
                        msg_success(
                                "Added " . $phonetype . " number '" . $phone .
                                "' for person '" . $name . "'.");
                    } else {
                        msg_error(
                                "Error adding number '" . $phone .
                                "' for person '" . $name . "': " . $mysqli->error);
                    }

                    break;
                } // otherwise, do nothing
                else {
                    msg_warn("'" . $name . "' is already in the phone book.");
                    break;
                }
            }

            // if the person is not there, add him
            // and create dummy phone if none specified
            $newphone = $phone ? $phone : "none";
            $q1 = "insert into person (Name) values ('" . $name . "')";
            $q2 = "set @id = (select PersonID from person where Name like ('" .
                    $name . "'))";
            $q3 = "insert into phone (PersonID, PhoneNumber, PhoneType) values (@id, '" .
                    $newphone . "', '" . $phonetype . "')";

            // queries okay?
            if (! ($mysqli->query($q1) and $mysqli->query($q2) and
                    $mysqli->query($q3))) {
                msg_error(
                        "Error adding '" . $name . "' to the phone book: " .
                        $mysqli->error);
                break;
            }

            // good; we are done
            msg_success(
                    "Added '" . $name . "' to the phone book" . ($newphone ==
                    'none' ? "." : " with " . $phonetype . " number " . $newphone .
                    "."));

            break;

        /* --------------------------------------------------------------- */

        case ('find'):

            // does exactly one of the fields have input?
            if (! ($name xor $phone)) {
                msg_warn("Enter either a name or a number to find.");
                break;
            }

            // pick query based on which field has input
            if ($name) {

                $info = $name;

                // includes wild cards
                $q1 = "select Name, PhoneNumber, PhoneType from phone, person where phone.PersonID = person.PersonID and Name like '%" .
                        $name . "%'";
            }

            if ($phone) {
                $info = $phone;
                $q1 = "select Name, PhoneNumber, PhoneType from phone, person where phone.PersonID = person.PersonID and PhoneNumber like '" .
                        $phone . "'";
            }

            // query okay?
            if (! ($result = $mysqli->query($q1))) {
                $msg = "Error searching for '" . $info . "': " . $mysqli->error;
                $msg_color = $col_error;
                break;
            }

            // good; we are done
            if (($nr = $result->num_rows) > 0) {
                msg_info(
                        "Displaying " . $nr . " result" . ($nr > 1 ? "s" : "") .
                        " for '" . $info . "'.");
            } else {
                msg_info("No results for '" . $info . "'.");
            }

            $listquery = $q1;

            break;

        /* --------------------------------------------------------------- */

        case ('delete'):

            if (! ($name || $phone)) {
                msg_warn("Enter a name and/or a number to delete.");
                break;
            }

            if ($phone && $name) {

                // if both a phone and a person are entered, delete the phone
                // for that person only
                $q1 = "set @id = (select PersonID from person where Name like ('" .
                        $name . "'))";
                $q2 = "delete from phone where PhoneNumber like '" . $phone .
                        "' and PersonID = @id";

                if (! ($mysqli->query($q1) && $mysqli->query($q2))) {
                    msg_error(
                            "Error deleting number '" . $phone . " for person '" .
                            $name . "': " . $mysqli->error);
                    break;
                }

                if ($mysqli->affected_rows > 0) {
                    msg_success(
                            "Removed number '" . $phone . "' for person '" .
                            $name . "'.");
                } else {
                    msg_warn(
                            "No name '" . $name . "' associated with number '" .
                            $phone . "'.");
                }

                break;
            } else {

                if ($name) {

                    $info = $name;
                    $q1 = "delete from person where Name like '" . $name . "'";
                }

                // if just a phone is entered, delete all instances of it
                if ($phone) {

                    $info = $phone;

                    // unfortunately we have to make sure everyone affected by
                    // the phone delete has at least a dummy phone number
                    // otherwise they won't show up.
                    // working around a poor design choice

                    $q1 = "select phone.PersonID from phone, person where phone.PersonID = person.PersonID and PhoneNumber like '" .
                            $phone . "'";

                    if ($result = $mysqli->query($q1)) {

                        while ($person_with_this_number = $result->fetch_array()) {
                            $id = $person_with_this_number[0];
                            $cnt = $mysqli->query(
                                    "select count(*) as c from phone where PersonID = " .
                                    $id)->fetch_object()->c;

                            // is this someone's only number?
                            if ($cnt == '1') {

                                // if so, then replace it with a dummy
                                if (! ($mysqli->query(
                                        "insert into phone values (" . $id .
                                        ", 'none', 'none')"))) {
                                    msg_error($mysqli->error);
                                    break;
                                }
                            }
                        }
                    }

                    $q1 = "delete from phone where PhoneNumber like '" . $phone .
                            "'";
                }

                if (! ($mysqli->query($q1))) {
                    msg_error(
                            "Error deleting " .
                            ($info == $name ? "person" : "phone") . " '" . $info .
                            "': " . $mysqli->error);
                    break;
                }
            }

            if ($mysqli->affected_rows > 0) {

                msg_success("Removed '" . $info . "' from the phone book.");
            } else {
                msg_warn("'" . $info . "' is not in the phone book.");
            }

            break;
        /* --------------------------------------------------------------- */
        case ('update'):

            // there is some javascript that changes the form placeholders
            // if 'update' is selected
            $oldname = $name;
            $newname = $phone;

            // is a name entered in both fields?
            if (! ($oldname && $newname)) {
                msg_warn("Enter a name and a new name to replace it with.");
                break;
            }

            // is the person in the phone book?
            $q1 = "select * from person where name like '" . $oldname . "'";
            if (! ($mysqli->query($q1)->num_rows > 0)) {
                msg_warn("'" . $oldname . "' is not in the phone book.");
                break;
            }

            // is the new name different from the old one?
            if ($oldname == $newname) {
                msg_info(
                        "New name is the same as old name; no information was changed.");
                break;
            }

            //
            if ($mysqli->query(
                    "update person set Name = '" . $newname . "' where name = '" .
                    $oldname . "'")) {
                msg_success(
                        "Changed name '" . $oldname . "' to '" . $newname . "'.");
            } else {
                msg_error("Error updating name: " . $mysqli->error);
            }

            break;
    }
}

display($listquery);

/* --------------------------------------------------------------- */
function msg_error ($msg)
{
    $GLOBALS['msg'] = $msg;
    $GLOBALS['msg_color'] = "#ffe3db";
}

function msg_success ($msg)
{
    $GLOBALS['msg'] = $msg;
    $GLOBALS['msg_color'] = "#deffdb";
}

function msg_info ($msg)
{
    $GLOBALS['msg'] = $msg;
    $GLOBALS['msg_color'] = "#f7fffe";
}

function msg_warn ($msg)
{
    $GLOBALS['msg'] = $msg;
    $GLOBALS['msg_color'] = "#fff9d4";
}

function display ($query)
{
    echo '<div class="msg" style="background-color: ' . $GLOBALS['msg_color'] . ';">
            ' . $GLOBALS['msg'] . '
            </div>';

    if ($result = $GLOBALS['mysqli']->query($query)) {

        if ($result->num_rows > 0) {

            echo '<table class="table table-bordered table-striped">
                  <tr> 
                  <th> Name   </th>
                  <th> Number </th>
                  <th> Type   </th>';

            $last = "";
            $name = "";
            $phone = "";
            $type = "";

            while ($row = $result->fetch_array(MYSQLI_ASSOC)) {

                echo '<tr>';

                if ($row["Name"] != $last) {
                    $last = $row["Name"];
                    $name = $last;
                } else {
                    $name = "";
                }

                if ($row["PhoneNumber"] != "none") {
                    $phone = $row["PhoneNumber"];
                    $type = $row["PhoneType"];
                } else {
                    $phone = "(No number)";
                    $type = "";
                }

                echo "<td> $name  </td>
                      <td> $phone </td>
                      <td> $type  </td>
                       </tr>";
            }

            echo "</table>";
        }

        $result->close();
    } else {
        echo "Error in query: " . $GLOBALS['mysqli']->error;
    }
}
/* --------------------------------------------------------------- */
?>
</div>
    </div>
</body>
</html>

Kate · Accepted Answer · 2020-11-07 23:21:07Z

Security

Security is super important. In 2020 it is not acceptable to write code that is blatantly vulnerable to SQL injection. Start taking good habits today.

Even if you are lazy and you are the only one using this application you should still fix the SQL injection vulnerabilities because your code will choke on single quotes - and single quotes in family names are not uncommon (eg O'hara). This code is broken anyway.

Don't show full error messages to users. In this case this would only help hackers by giving them insight into your database structure, and help them take over your site.

I am also wondering how your form will behave if Javascript is not enabled on the browser ? It may degrade or not work properly. At the very least it should not cause security issues or lead to erratic behavior impacting the data.

CSS

I see that you are using Bootstrap but we are at version 4 right ? Consider bringing your code up to date while you are in the early stages of design. Then you have some more CSS, I suppose it is a mix of additional styles and Bootstrap overrides. I suggest that you move all your CSS to a separate .css file so as to declutter your file (= less scrolling, less distraction). It should come last if it contains Bootstrap overrides. Then you'll have more than 70 lines of code out of your sight.

You also have some inline CSS like:

<div class="col-xs-5" style="padding: 1px">

repeated several times, then it would make sense to redefine styles col-xs-5 and col-xs-1 instead. This will avoid lots of search & replace if you want to tweak the layout in the future. The idea is to avoid repetition.

Same remark about your Javascript: it can go to an external file, and you reduce your overall code by another 30 lines.

An added benefit is that your browser can keep the external files in cache.

DB connection

For the database connection you might keep it in an include file, especially if the code is going to be reused in other pages.

Performing deletes

I can't test your code locally but I think some of the logic is flawed, for example the delete:

        $q1 = "set @id = (select PersonID from person where Name like ('" .
                $name . "'))";
        $q2 = "delete from phone where PhoneNumber like '" . $phone .
                "' and PersonID = @id";

First you fetch the ID of the person based on the name, then you proceed to delete a phone number belonging to that person. But what happens if you have two persons with the same name ? In this case the query would fail because it returns more than one row. In other cases you might destroy data because of the criteria returning more records than intended.

Normally, you would keep a database ID for each phone number in a hidden field in your form, and this is what you send in your POST request. The delete should then be performed on the basis of that ID. The two-step process is redundant and unsafe.

Same remark for deleting a person, provide an ID that is unique as a criterion.

You don't even need a LIKE if you are not using % for pattern matching, just use =.

Orphaned users

To get round this problem:

           // unfortunately we have to make sure everyone affected by
            // the phone delete has at least a dummy phone number
            // otherwise they won't show up.
            // working around a poor design choice

            $q1 = "select phone.PersonID from phone, person where phone.PersonID = person.PersonID and PhoneNumber like '" .
                    $phone . "'";

It seems to me that you just need an OUTER JOIN or phone.PersonID =* person.PersonID. Feeding your database with garbage data is indeed not a good thing.

Use transactions

You should use transactions if you perform multiple, related operations in one batch. If one statement fails in the middle for any reason, you will end up with inconsistent data or orphaned records.

Reserved keywords

Look at this query again:

    $q1 = "set @id = (select PersonID from person where Name like ('" .
            $name . "'))";

Name is a reserved keyword in Mysql, it should be avoided for naming objects. It's still possible to enclose them within brackets or backticks but not recommended.

\$\begingroup\$ Start taking good habits today. I wish everyone would :) \$\endgroup\$
– AyCe
Commented Nov 8, 2020 at 4:24 — AyCe, Commented Nov 8, 2020 at 4:24

htl · Accepted Answer · 2020-11-08 10:24:29Z

Your project looks nice, and the design of your web page is good. There were a few points I spotted with the code that are well worth taking note of.

Security

While security may sound less important if you're the only one using the project, the issues below are both critical for any public-facing website, and affect the correctness of the program as pointed out by ComFreek and the other answers. Your site should be able to cope with any user input, even that deliberately designed to cause problems, in order to be fully correct.

It's useful to stick to best practices for any project so you remember to do things the safe way.

Your code appears to be vulnerable to SQL injection. Whenever you want to perform a database query with user input, you cannot trust that the data the user gives is safe to put straight into a query. You should consider using prepared statements and the PDO extension of PHP. If you don't do this, a malicious user could choose a name like ');drop table person;#. This would evaluate your query as
```
select PersonID from person where Name like ('');drop table person;#')
                                              ----------------------
```
so the attacker has now deleted your entire person database. The dashes show you where I substituted the name into the query, as PHP would.
Your code also seems vulnerable to cross-site scripting (XSS). Suppose a user entered a name along the lines of
```
<script> alert("Hello, world"); </script>
```
Your web page then embeds this into the HTML, so your browser will run the script. This can be exploited in all sorts of ways, and you'll find many examples on the web where this type of attack has been successful. One option is to ensure any user input you display has been sanitised with htmlspecialchars. You could, for example, escape the data before inputting it into the database. It's very easy to forget this, so a more robust option is to use a templating engine such as Twig. This will automatically escape any variables you display unless you specifically ask not to do this.
Storing your database connection credentials in the code is often discouraged for production-level code. See, for example this Security Stack Exchange post on the topic. Instead, store the credentials in a configuration file, or as an environment variable.

I disagree with the sentiment that the security-related suggestions aren't important if OP is the only one using the app. SQL injections and XSS are not mere vulnerabilities -- they make the code incorrect. For example, entering any text value with quotes or HTML syntax will lead to query or UI errors. Apart from that major point, you also want to never get into the habit of coding like this lest you repeat the errors again and again in production systems. — ComFreek, Commented Nov 8, 2020 at 10:15
@ComFreek That is a very good point regarding the correctness, thanks. I've reconsidered my statement on that because as you rightly point out, even if you didn't care about security (which you certainly should) the code will still fail on real data that could genuinely be put into the site. — htl, Commented Nov 8, 2020 at 10:27
Good suggestion to use Twig. If you later develop in Python and Flask for example you will notice the similarities with the Jinja templating engine. — Kate, Commented Nov 8, 2020 at 14:37

Ismael Miguel · Accepted Answer · 2020-11-08 23:30:49Z

I've noticed that you are using Bootstrap 3.3.6, which isn't maintained anymore.

The latest 3.x version is 3.4.1, and you should consider using it.

However, you're doing something wrong:
You are misusing the framework

Below is a copy part of the HTML, with all the CSS:

body {
    font-family: Ubuntu Mono;
    background-color: #F2F1F0;
}

.form-control {
    border-radius: 0;
    border: 1px solid #AEA79F;
}

.btn-default {
    background-color: #EEEDEB;
}

.form-control:focus {
    border: 1px solid #E95420 !important;
    box-shadow: none !important;
}

.btn:focus, .btn:active {
    outline: none !important;
    box-shadow: none;
}

.header {
    text-shadow: 0px 1px #666;
    box-shadow: 0 0px 0 #F6BBA6 inset;
    color: #F6F6F5;
    background-color: #E95420;
    font-size: 28px;
    font-weight: bold;
    padding: 1px;
    padding-left: 20px;
    border: 1px solid #555;
    margin-top: 12px;
}

.c2 {
    background-color: #FFF;
    border: 1px solid #666;
    border-top: none;
    padding: 18px;
}

.msg {
    border: 1px solid #AEA79F;
    border-style: dashed;
    padding: 8px;
    margin-bottom: 18px;
}

.table td, th {
    border: 1px solid #AEA79F !important;
    border-style: dotted !important;
}

.spc {
    padding: 18px;
}

.inv {
    display: none;
}

input[type="radio"] {
    margin: 5px;
    margin-top: 4px;
    vertical-align: middle;
}

input:invalid {
    border: 1px solid red;
}

<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css">

<link rel="stylesheet" href="//fonts.googleapis.com/css?family=Ubuntu+Mono:300,400,600,700&amp;lang=en" />

<div class="container">

    <div class="header">Phone Book</div>

    <div class="c2">

        <form action="PhoneForm.php" method="post">

            <div class="col-xs-5" style="padding: 1px">
                <input type="text" class="form-control" name="name" id="name"
                    placeholder="Name" maxlength="36">

            </div>
            <div class="col-xs-5" style="padding: 1px;">

                <input type="text" class="form-control" name="phone" id="phone"
                    placeholder="Phone Number" maxlength="17">

                <div class="phonetypes" class="inv">
                    <input type="radio" name="phonetype" value="other" CHECKED>Other<input
                        type="radio" name="phonetype" value="home">Home <input
                        type="radio" name="phonetype" value="work">Work <input
                        type="radio" name="phonetype" value="cell">Cell <input
                        type="radio" name="phonetype" value="fax">Fax
                </div>

            </div>
            <div class="col-xs-1" style="padding: 1px">
                <select class="form-control" id="actions" name="actions">
                    <option value="add">Add</option>
                    <option value="find" selected>Find</option>
                    <option value="delete">Delete</option>
                    <option value="update">Update</option>
                    <option value="viewall">View All</option>;
                </select>
            </div>

            <div class="col-xs-1" style="padding: 1px">
                <input type="submit" class="btn btn-default form-control"
                    name="btn" value="Go">
            </div>
      <br/>
      
            <div class="spc"></div>

        </form>
        
        <div class="msg" style="background-color: #ffe3db;">
            An error has happened
        </div>
    </div>
</div>

Problem 1: Where's the `.row`?

You have a <div class="container"> but you aren't using any <div class="row">.

Problem 2: inline CSS.

I understand: you want to remove all the spacing between the form elements.

Consider leaving it as-is.

Problem 3: more missuse

You are using a responsive framework, but your form isn't responsive at all.

Consider using the other form sizing classes.

For example, at the size of a tablet, you might want to make it have 2 rows of inputs instead of just 1.

Problem 4: you don't use already existing components

You create a <div class="msg">...</div> for the error messages.

Instead, you can use an alert which already exists in the plataform:

<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css">

<link rel="stylesheet" href="//fonts.googleapis.com/css?family=Ubuntu+Mono:300,400,600,700&amp;lang=en" />

<div class="alert alert-danger" role="alert">
  <strong>Error:</strong> Something bad happened
</div>

<div class="alert alert-warning alert-dismissible" role="alert">
  <button type="button" class="close" data-dismiss="alert" aria-label="Close"><span aria-hidden="true">&times;</span></button>
  <strong>Warning:</strong> Something less bad happened
</div>

<small>If you use bootstrap.js, you can do this too, and the close button works<small>

You can then personalize to your liking, and place where you want.

Conclusion

This should clean up your HTML and CSS quite a lot:

body {
    font-family: "Ubuntu Mono";
    background-color: #F2F1F0;
}

.form-control {
    border-radius: 0;
    border: 1px solid #AEA79F;
}

.btn-default {
    background-color: #EEEDEB;
}

.form-control:focus {
    border: 1px solid #E95420 !important;
    box-shadow: none !important;
}

.btn:focus, .btn:active {
    outline: none !important;
    box-shadow: none;
}

.header {
    text-shadow: 0px 1px #666;
    box-shadow: 0 0px 0 #F6BBA6 inset;
    color: #F6F6F5;
    background-color: #E95420;
    font-size: 28px;
    font-weight: bold;
    padding: 1px;
    padding-left: 20px;
    border: 1px solid #555;
    margin-top: 12px;
}

.c2 {
    background-color: #FFF;
    border: 1px solid #666;
    border-top: none;
    padding: 18px;
}

.table td, th {
    border: 1px solid #AEA79F !important;
    border-style: dotted !important;
}

.spc {
    padding: 18px;
}

input[type="radio"] {
    margin: 5px;
    margin-top: 4px;
    vertical-align: middle;
}

input:invalid {
    border: 1px solid red;
}

<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css">

<link rel="stylesheet" href="//fonts.googleapis.com/css?family=Ubuntu+Mono:300,400,600,700&amp;lang=en" />

<div class="container">

    <div class="header">Phone Book</div>

    <form action="PhoneForm.php" method="post" class="c2">
        <div class="row">
            <div class="col-xs-6 col-sm-4 col-md-5">
                <input type="text" class="form-control" name="name" id="name" placeholder="Name" maxlength="36">

            </div>
            
            <div class="col-xs-6 col-sm-3 col-md-3">

                <input type="text" class="form-control" name="phone" id="phone" placeholder="Phone Number" maxlength="17">

                <div class="phonetypes">
                    <input type="radio" name="phonetype" value="other" checked>Other
                    <input type="radio" name="phonetype" value="home">Home
                    <input type="radio" name="phonetype" value="work">Work
                    <input type="radio" name="phonetype" value="cell">Cell
                    <input type="radio" name="phonetype" value="fax">Fax
                </div>
            </div>
            
            <div class="col-xs-6 col-sm-3 col-md-2">
                <select class="form-control" id="actions" name="actions">
                    <option value="add">Add</option>
                    <option value="find" selected>Find</option>
                    <option value="delete">Delete</option>
                    <option value="update">Update</option>
                    <option value="viewall">View All</option>;
                </select>
            </div>
            
            <div class="col-xs-6 col-sm-2 col-md-2">
                <input type="submit" class="btn btn-default form-control" name="btn" value="Go">
            </div>
        </div>
        
        <div class="spc"></div>
        
        <div class="alert alert-danger" role="alert">
            <strong>Error:</strong> Something bad happened
        </div>
    </form>
</div>

I've re-used the form as a .row, which is fine, but should be avoided.
I've done it just to avoid another level or indentation.

This is all just an example of how the code may look like, not a 200% final implementation.

Despite all the negativity, it shows that you've tried to learn about Bootstrap (you use the correct classes for the table, instead of re-implementing what already exists).

Other issues

Adding to other answers, you still have some issues.

Problem 1: global variables

While not exactly wrong to do, there are better ways to do things.

In this case, no global variables.

If you want to store all messages before showing them, try making a class like this:

<?php

class Alert {
    private static $messages = array();
    
    public static function error($message, $can_dismiss = false)
    {
        self::add('danger', $message, $can_dismiss);
    }
    
    public static function warning($message, $can_dismiss = false)
    {
        self::add('warning', $message, $can_dismiss);
    }
    public static function info($message, $can_dismiss = false)
    {
        self::add('info', $message, $can_dismiss);
    }
    public static function success($message, $can_dismiss = false)
    {
        self::add('success', $message, $can_dismiss);
    }
    
    public static function add($class, $message, $can_dismiss)
    {
        self::$messages[] = array(
            'dismiss' => !!$can_dismiss,
            'class' => $class,
            'message' => $message
        );
    }
    
    public static function render()
    {
        foreach(self::$messages as $message)
        {
            echo '<div class="alert alert-', $message['class'], ($message['dismiss'] ? ' alert-dismissible' : ''),'" role="alert">',
                ($message['dismiss'] ? '<button type="button" class="close" data-dismiss="alert" aria-label="Close"><span aria-hidden="true">&times;</span></button>' : ''),
                $message['message'],
            '</div>';
        }
    }
}

To use it, you can just do Alert::error() or you can do Alert::add(<type>, <message>).

You can call Alert::render() even if you don't have any messages to show.

Remember: this is an example of how you can do. It works, but isn't a 100% final version.

Problem 2: `function display ($query)` does too much

Currently, the display function has to do all the query fetching, error handling, presentation and query cleaning up.

You should just do one thing with it: pass to it the array with all the things to display.

Do everything else outside that function.

There are other issues with the code, but, those are sorta minor ("wrong" operators, weird queries, ...) and can be for some other time (maybe a version 2?).

Stack Exchange Network

Reorganizing unorthodox PHP phone book project

3 Answers 3

Security

CSS

DB connection

Performing deletes

Orphaned users

Use transactions

Reserved keywords

Security

Problem 1: Where's the `.row`?

Problem 2: inline CSS.

Problem 3: more missuse

Problem 4: you don't use already existing components

Conclusion

Other issues

Problem 1: global variables

Problem 2: `function display ($query)` does too much

Your Answer

Not the answer you're looking for? Browse other questions tagged
javascript
php
html
sql
or ask your own question.

Hot Network Questions

Reorganizing unorthodox PHP phone book project

3 Answers 3

Security

CSS

DB connection

Performing deletes

Orphaned users

Use transactions

Reserved keywords

Security

Problem 1: Where's the .row?

Problem 2: inline CSS.

Problem 3: more missuse

Problem 4: you don't use already existing components

Conclusion

Other issues

Problem 1: global variables

Problem 2: function display ($query) does too much

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged javascriptphphtmlsql or ask your own question.

Related

Hot Network Questions

Problem 1: Where's the `.row`?

Problem 2: `function display ($query)` does too much

Not the answer you're looking for? Browse other questions tagged
javascript
php
html
sql
or ask your own question.