2
\$\begingroup\$

I want to load some CSV columns or contents in my Ajax code. Please help me or improve my code. This will be live API code for me.

Data will be downloaded from Yahoo Finance in CSV format on an interval, and updated into this chart code. The values will be updated and chart will continue to be updated.

<?php $row = 1; if(($handle = fopen("table.csv", "r")) !== FALSE)  {
while (($data = fgetcsv($handle, 1000, ",")) !== FALSE){
$row++;
for ($c=0; $c<1; $c++) 
{
echo $data[$c];   
}         
for ($c1=5; $c1<6; $c1++) 
{
$data[$c];                   
}
for ($c2=4; $c2<5; $c2++)                    
{
$data[$c];
}
fclose($handle); 
} 
?>


<script type="text/javascript" src="http://www.stoquity.com/js/jazaa/amcharts.js"></script>        
<script type="text/javascript" src="http://www.stoquity.com/js/jazaa/raphael.js" type="text/javascript"></script>



var chart;
var chartData = [

{   year: '(here is the data from csv "col 1 row 1")',  income: '(here is the data from csv "col 5 row 1")',     expenses: '(here is the data from csv "col 4 row 1")'},{ year: '(here is the data from csv "col 1 row 2")',    income: '(here is the data from csv "col 5 row 2")',     expenses: '(here is the data from csv "col 4 row 2")'},{year: '(here is the data from csv "col 1 row 3")',    income: '(here is the data from csv "col 5 row 3")',expenses: '(here is the data from csv "col 3 row 3")'}];


AmCharts.ready(function () {
        // SERIAL CHART  
        chart = new AmCharts.AmSerialChart();
        chart.pathToImages = "../amcharts/images/";
        chart.dataProvider = chartData;
        chart.categoryField = "year";
        chart.startDuration = 1;

        // AXES
        // category
        var categoryAxis = chart.categoryAxis;
        categoryAxis.gridPosition = "start";

        // value
        // in case you don't want to change default settings of value axis,
        // you don't need to create it, as one value axis is created automatically.

        // GRAPHS
        // column graph
        var graph1 = new AmCharts.AmGraph();
        graph1.type = "column";
        graph1.title = "Income";
        graph1.valueField = "income";
        graph1.lineAlpha = 0;
        graph1.fillAlphas = 1;
        chart.addGraph(graph1);

        // line
        var graph2 = new AmCharts.AmGraph();
        graph2.type = "line";
        graph2.title = "Expenses";
        graph2.valueField = "expenses";
        graph2.lineThickness = 2;

        chart.addGraph(graph2);

        // LEGEND                

        // WRITE
        chart.write("chartdiv");
    });
</script>
</head>

<body>
<div id="chartdiv" style="width:850px; height:300px;"></div>
</body>
\$\endgroup\$

2 Answers 2

Reset to default
3
\$\begingroup\$

Did you read or even try this code before posting it? Sorry if this seems rude, but it just seems obvious that if this was run you would immediately notice something was wrong. I'm mostly looking at the PHP code here, as it is full of issues. The JS appears to be fine, except I would consider moving those graphs into a separate functions. More on that below.

PHP

Wow, so many things on just the first line.

Only do one thing per line, this helps with legibility. Also, try to avoid putting statements on the same line as the opening or closing php tags, unless opening and closing them on the same line (templates usually).

Don't assign variables in statements (if, while, etc...) as it can lead to mistakes. PHP allows this, but IMO they should deprecate it. Though they probably won't because it might cause issues with the way for loops are declared. It is very easy to forget a second equals sign and mistakenly assign a new value to a variable that you were actually trying to compare, and your IDE wont catch it because its valid. If you get in the habit of not doing this then you know that if you ever see it while troubleshooting you know you've found your issue. And if your IDE is smart enough it will warn you about accidental assignments.

Double quotes are used to tell PHP that the following string has a character or variable that needs to be escaped. As such PHP requires a little bit of extra processing power to read that string than if it were just single quoted. The speed is really negligible and is not really a "good" reason for making this switch, but I like to point it out anyways. This is a very controversial issue, so take it or leave it.

The first line rewritten:

<?php
$row = 1;
$handle = fopen( 'table.csv', 'r' );
if( $handle !== FALSE )  {

Correct me if I'm wrong, but doesn't your for loop just print once on each while iteration? In this case, why loop at all? The same thing can be accomplished with array_shift() or by specifying the first index.

echo array_shift( $data );
//or to avoid manipulating array
echo $data[ 0 ];

Why are all of your for loops only looping once? What is their purpose? Let me read you your code and you can follow along. Starting inside the while loop and doing the following on each iteration.

  1. Increment unused row variable
  2. Loop once
    1. Echo first element of new CSV array
    2. End of loop, increment a counter that won't really be used
  3. Loop once with a specific number that's not used
    1. Use first counter to point at the second value of array, Do nothing with it
    2. End of loop, increment a counter that isn't used
  4. Loop once with another specific number that's not used
    1. Use first counter to point at the second value of array, Do nothing with it
    2. End of loop, increment a counter that isn't used
  5. Close file early so that only first entry was retrieved and while loop ends early

I'm not sure what those other loops are being used for, but If you want to do something every 5th iteration just add an if statement with a mod of 5.

if( $c % 5 === 0 ) {

What is the purpose of $row, $c1, and $c2? These variables are unused. And What is the purpose of just declaring $data[ $c ] in each of these loops, it doesn't do anything. At least the first loop echoed it.

Why are you closing your handle while you are still using the file? Close it after the loop unless you meant to only fetch the first entry.

JS

You can reduce the amount of work you are doing by following the "Don't Repeat Yourself" (DRY) Principle. You should also look at the Single Responsibility Principle to break up that ready function a little more. Now, my JS is a little rusty, but here's how I'd apply the DRY principle. I have not tested it, but hopefully it gets the point across.

function graph( properties ) {
    graph = new AmCharts.AmGraph();
    for( property in properties ) {
        graph[ property ] = properties[ property ];
    }

    return graph;
}

graph1Properties = {
    type : "column",
    title : "Income",
    valueField : "income",
    lineAlpha : 0,
    fillAlphas : 1
};
graph1 = graph( graph1Properties );
chart.addGraph( graph1 );

graph2Properties = {
    type : "line",
    title : "Expenses",
    valueField : "expenses",
    lineThickness : 2
};
graph2 = graph( graph2Properties );
chart.addGraph( graph2 );
\$\endgroup\$
0
\$\begingroup\$

i am understanding you. but my actual probalme is that, there is a csv file. i want to fetch data from that csv file to AJAX variable.

var chartData = [

{ year: '(here is the data from csv "col 1 row 1")', income: '(here is the data from csv "col 5 row 1")',
expenses: '(here is the data from csv "col 4 row 1")'},

{ year: '(here is the data from csv "col 1 row 2")',
income: '(here is the data from csv "col 5 row 2")',
expenses: '(here is the data from csv "col 4 row 2")'},

{year: '(here is the data from csv "col 1 row 3")',
income: '(here is the data from csv "col 5 row 3")', expenses: (here is the data from csv "col 3 row 3")},

]

but continuously 254 time

\$\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.