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

asked Aug 24, 2017 at 10:57
\$\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)?
mickmackusa
8,8021 gold badge17 silver badges31 bronze badges
answered Aug 24, 2017 at 20:23
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.