2
\$\begingroup\$

My code is basically the home page (index.php) of a website for finding products. It accesses a database table to get a list of all countries and then populates a <select> so the user can add a country to their search filter while finding products.

  1. Are there any website "code smells" in this code?
  2. Is it considered clean to have a big PHP block to access the database inside the HTML? It seems kind of like I am mixing logic with presentation.
  3. Can my PHP database query be shifted into a separate file somehow?
  4. Can the tiny <script type="text/javasript"> tag somehow be put into a separate file?
  5. Is it bad practice to have a global JavaScript variable called allCountries inside index.php so that I can access it in country.ts?

index.php:

<!-- This is the home page -->
<html>
    <head>
        <title>The Vegan Repository</title>
        <meta charset="utf-8">
        <meta name="viewport" content="width=device-width, initial-scale=1">
        <!-- Load libraries -->
        <!-- System.js -->
        <script 
            src="https://cdnjs.cloudflare.com/ajax/libs/systemjs/0.18.4/system-csp-production.js"></script>
        <!-- JQuery -->
        <script 
            src="bower_components/jquery/dist/jquery.min.js"></script>
        <!-- Bootstrap CSS -->
        <link 
            rel="stylesheet" 
            href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css" 
            integrity="sha384-1q8mTJOASx8j1Au+a5WDVnPi2lkFfwwEAa8hDDdjZlpLegxhjVME1fgjWPGmkzs7"
            crossorigin="anonymous">
        <!-- Bootstrap JS -->
        <script 
            src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/js/bootstrap.min.js" 
            integrity="sha384-0mSbJDEHialfmuBBQP6A4Qrprq5OVfW37PRR3j5ELqxss1yVqOtnepnHVP9aJ7xS"
            crossorigin="anonymous"></script>
        <!-- Knockout JS -->
        <script 
            src="https://cdnjs.cloudflare.com/ajax/libs/knockout/3.4.0/knockout-min.js"></script>
        <!-- Styles -->
        <link rel="stylesheet" type="text/css" href="styles/image.css">
        <link rel="stylesheet" type="text/css" href="styles/text.css">
        <link rel="stylesheet" type="text/css" href="styles/header.css">
        <link rel="stylesheet" type="text/css" href="styles/form.css">
        <link rel="stylesheet" type="text/css" href="styles/select.css">
        <?php
            $host = 'localhost';
            $port = 8889;
            $servername = "$host:$port";
            $username = "root";
            $password = "root";
            $db = "myDB";

            // Create connection
            $conn = new mysqli($servername, $username, $password, $db);
            // Check connection
            if ($conn->connect_error) {
                die("Connection failed: " . $conn->connect_error);
            } 

            $sql = "SELECT name, geonameId FROM countryinfo";
            $result = $conn->query($sql);
            $allCountries = [];
            if ($result->num_rows > 0) {
                $rows = array ();
                while($row = $result->fetch_assoc()) {
                    $country = array(
                        "name" => $row['name'],
                        "geonameId" => $row['geonameId']
                    );
                    $allCountries[] = $country;
                }
            }
            $conn->close();
            ?>
        <script type="text/javascript">
            var allCountries = <?php echo json_encode($allCountries, JSON_PRETTY_PRINT) ?>;
        </script>
    </head>
    <body >
        <div style="margin-bottom: 100px;" class="full_size dimmed">
            <div style="position:fixed; z-index: -1;">
                <video 
                    style="position:fixed;" 
                    autoplay loop muted
                    poster="assets/images/home_page/polina.jpg" 
                    id="bgvid">
                    <!--<source src="polina.webm" type="video/webm">-->
                    <source src="assets/videos/polina.mp4" type="video/mp4">
                </video>
            </div>
            <div class="header dim">
                <a href="http://www.w3schools.com" >
                    <h5 id="app-name" class="nav-item clickable white-text medium-text left-text">THE VEGAN REPOSITORY</h5>
                </a>
                <a href="http://www.w3schools.com" >
                    <h5 (click)="clicked()" id="sign-in-button" class="nav-item clickable brand-colour-text medium-text right-text with-border">SIGN UP FREE</h5>
                </a>
                <a href="http://www.w3schools.com" >
                    <h5 class="nav-item clickable white-text medium-text right-text">LOGIN</h5>
                </a>
                <a href="#home_page_footer" >
                    <h5 class="nav-item clickable white-text medium-text right-text" >BLOG</h5>
                </a>
                <a href="#home_page_footer" >
                    <h5 class="nav-item clickable white-text medium-text right-text" >ABOUT</h5>
                </a>
            </div>
            <div id="motto-text" class="vertical-center">
                <h5 class="white-text medium-text">THE VEGAN REPOSITORY</h5>
                <h1 id="main-text" class=" text-center white-text light-text extra-large-text">FIND VEGAN PRODUCTS NEAR YOU</h1>
                <a id="try-now-button" class="with-border clickable" href="#find-vegan-products-page" >
                    <h5  class=" text-center medium-text">TRY NOW</h5>
                </a>
            </div>
        </div>
        <!--[if lt IE 9]>
        <script>
            document.createElement('video');
        </script>
        <![endif]-->
        <div id="find-vegan-products-page" style="height:900px;">
            <div class="form-background">
                <div class="container-fluid" style="padding: 40px;">
                    <h1>Filter Your Search!</h1>
                    <form role="form">
                        <div class="row">
                            <div class="form-group col-sm-6">
                                <div class="select">
                                    <span class="arr"></span>
                                    <select data-bind="options: _countries,
                                        optionsText: 'name',
                                        optionsValue: 'geonameId',
                                        value: selectedCountry,
                                        optionsCaption: 'Country'">
                                    </select>
                                </div>
                            </div>
                            <div class="form-group col-sm-6">
                                <div class="select">
                                    <span class="arr"></span>
                                    <select data-bind="options: _regions,
                                        optionsText: 'name',
                                        optionsValue: 'name',
                                        value: selectedCity,
                                        optionsCaption: 'Region'">
                                    </select>
                                </div>
                            </div>
                        </div>
                    </form>
                </div>
            </div>
        </div>
        </div>
        <div id='container'>
            <div>always showing</div>
            <div id='loading' data-bind="visible: isLoading">Loading...</div>
        </div>
        <div>
            <button data-bind="click: loadData">Simulate Load
        </div>
        </div>
        <script src="scripts/scroll_to_anchor.js"></script>
        <script src="scripts/xml2json.min.js"></script>
        <script src="scripts/country.js"></script>
    </body>
</html>

country.ts (TypeScript):

class HomeViewModel {
    _countriesList = [];
    _regionsList = [];
    _countries = ko.observableArray();
    _regions = ko.observableArray();
    selectedCountry = ko.observable();

    constructor(allCountries) {
        for (var index = 0; index < allCountries.length; index++) {
            this._countriesList.push(allCountries[index]);
        }
        this._countries(this._countriesList);
        this.selectedCountry.subscribe(this.updateCities.bind(this))
    }

    updateCities(geonameId) {
        var self = this;
        self._regionsList = [];
        $.ajax({
            url: `http://api.geonames.org/children?geonameId=${geonameId}&username=elion`
        }).then(function(allCitiesXML) {
            var allCitiesJSON = xml2json(allCitiesXML);
            var allCities = JSON.parse(allCitiesJSON);
            for (var index = 1; index < allCities.geonames.length - 1; index++) {
                self._regionsList.push(allCities.geonames[index].geoname);
            }
            self._regions(self._regionsList);
        });
    }
}

$(document).ready(function() {
    var _homeViewModel: HomeViewModel = new HomeViewModel(allCountries);
    ko.applyBindings(_homeViewModel);
);
\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

you can separate your db connection in some db.php file and include it on this

page "include('db.php')"
$host = 'localhost';
$port = 8889;
$servername = "$host:$port";
$username = "root";
$password = "root";
$db = "myDB";
// Create connection
$conn = new mysqli($servername, $username, $password, $db);
// Check connection
if ($conn - > connect_error) {
    die("Connection failed: ".$conn - > connect_error);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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