0
\$\begingroup\$

The Code

PHP:

<?php 
session_start();

if ($_SESSION['Username'] == "") 
{
    header('Location: Login.php');
}
?>

Java Script:

<script type="text/javascript" src="https://www.gstatic.com/charts/loader.js"></script>
<script type="text/javascript">
    //Google Stuff
    google.charts.load('current', {packages: ['corechart']});
    google.charts.setOnLoadCallback(drawChart);

    function drawChart() {
        // Define the chart to be drawn.
        var data = new google.visualization.DataTable();
        data.addColumn('string', 'Element');
        data.addColumn('number', 'Percentage');
        data.addRows([          
            <?php

            include 'php/dbh.php';

            $Rows = array();

            $sql = "SELECT * FROM Sales";

            $Result = mysqli_query($conn, $sql);

            if ($Result) 
            {
                while ($row = mysqli_fetch_assoc($Result)) {
                    $Rows[] = $row;
                }
            }
                $ListArray = array();
                foreach ($Rows as $row) 
                {
                    asort($row);
                    $row = explode(",", $row['Products']);
                    $ListArray = $row;

                }


                $numItems = count($ListArray);
                $i = 0;
                foreach ($ListArray as $item) 
                {
                    $i2 = 0;
                    $Newitem = ltrim($item);
                    $sql2 = "SELECT * FROM submenus WHERE Name='".$Newitem."'";
                    $Result = mysqli_query($conn, $sql2);
                    if (!$Result) 
                    {
                        print_r(mysqli_error($conn));
                    }
                    if ($Result) 
                    {
                        while ($row = mysqli_fetch_assoc($Result)) 
                        {
                            $i2 = $row['Sales'];

                        }
                    }

                    if(++$i === $numItems) 
                    {
                        echo "['".$item."',".$i2."]";
                    }
                    else
                    {
                        echo "['".$item."',".$i2."],";
                    }                           
                }

                ?>
            ]);

        // Set chart options
    var options = {
    title:'Sales',
    titleTextStyle: {color: 'white', bold: true},
    height:300,
    backgroundColor: '#3a3939',
    color: '#fff',
    legend: {textStyle: {color: 'white'}},
    hAxis: {gridlines: {color: 'black'}}};

  // Instantiate and draw the chart.
  var chart = new google.visualization.PieChart(document.getElementById('SalesChar'));
  chart.draw(data, options );
    }

   </script>

HTML:

    <fieldset class="Sales">
        <legend style="color: white;">Sales</legend>
        <div id="SalesChar"/>
    </fieldset>

How it works

The PHP Code just start a session and if the Username is == "" it will redirect to the login page.

The Java Script is Located in the header of a webpage and it simply load and draw rows, inside the data.addRows([]); There is a PHP code that first gets a table called Sales, then for each row it will explode the column called products so it gets multiple string and then add it to a array then for each string it will get from another table called submenus then if everything goes right it will echo something like this ['Test', 2]

The Html is located in the body and it is only there because it is indicating where the chart should be drawn

What i want to know

i want to know if there is any way possible to make the PHP that is located inside the javascript more compact and more readable (i mean easier to understand) and take a less space.

I am open to any other suggestion

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Is the javascript part of the same file as the PHP? If so, you should show them together as a single file, not as separate pieces of code.


Typically, you should have already made a call to the DB to get records before you ever begin rendering things to the browser like you are doing for your javascript. Keep these pieces of logic separated. Your page might look more like:

<?php
// not-shown: file bootstrapping logic, if any
$rows = [];
// not shown: your DB logic to retrieve records

if(empty($rows)) {
    // you have no results to work with so perhaps handle this as error condition and exit/redirect.
}
?>
<script type="text/javascript" src="https://www.gstatic.com/charts/loader.js"></script>
<script type="text/javascript">
    // use json_encode to pass data from PHP to javascript literal
    var rows = <?php echo json_encode($rows); ?>;

    //Google Stuff
    google.charts.load('current', {packages: ['corechart']});
    google.charts.setOnLoadCallback(drawChart);

    function drawChart() {
        // Define the chart to be drawn.
        var data = new google.visualization.DataTable();
        data.addColumn('string', 'Element');
        data.addColumn('number', 'Percentage');
        data.addRows(rows);         

        // Set chart options
        var options = {
            title:'Sales',
            titleTextStyle: {color: 'white', bold: true},
            height:300,
            backgroundColor: '#3a3939',
            color: '#fff',
            legend: {textStyle: {color: 'white'}},
            hAxis: {gridlines: {color: 'black'}}
        };

        // Instantiate and draw the chart.
        var chart = new google.visualization.PieChart(document.getElementById('SalesChar'));
        chart.draw(data, options );
    }
</script>

Note my use of json_encode() to serialize the PHP data structure into javascript in the code above. This is the #1 most useful way to interchange data between PHP and javascript. Embrace the power of JSON serialization to simplify data transfer. Dynamically building javascript data structures via PHP string concatenation is a really bad approach.


I would encourage to look at working with mysqli in an object-oriented manner. There are a lot of legacy procedural PHP code examples out there, and while it is good to understand procedural style, I would venture to guess that you will likely fall into good development habits more easily if you bias yourself towards object-oriented programming for PHP.


Your SQL has some problems:

  • Don't use SELECT *. It is lazy and a potential point for application fragility. Why make a developer go look at a database table in order to understand what fields are available to a particular section of code? Why waste extra bandwidth sending across fields that are not needed in a certain area of code?
  • You have fallen into a querying-in-a-loop anti-pattern. Almost always, when you see code like this, it should be a red flag to you that perhaps a join is a better option than sending a bunch of individual queries against a database.
  • You may have a schema problem. Why work with Sales/Products as a comma-separated list? Should this be normalized into its own table (potentially with aggregation into comma-separated values if needed for purposes of a given query)?
\$\endgroup\$

Your Answer

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

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