1
\$\begingroup\$

Should I be using getter and setter functions in my class nodeStruct?

I am currently using them where there is need to manipulate the input value (e.g for $nodeID). Is this the way to go in php? Note that I am using the class mainly as a replacement for the not existent struct data type.

Also note that the script is currently being refactored (e.g introducing a class instead of declaring variables on the fly). The database that is accessed uses some columns for multiple variables (e.g v1;v2;v3 in column X), that's the reason for the strtok usage.

I am also well aware of the fact that the mysql API is gonna be deprecated in PHP7 and that the SQL queries that are currently made are not parameterized and therefore vulnerable to SQL-Injections.

Take a close look at the $monthSum in class nodeStruct array: A $unitScaleFactor is applied on it, and I would like to know if that is a thing you put in a setter function of the class or not. I am currently using the applyScale() function to achieve the correct scaling, it would seem intuitiv to just multiply the values when setting them.

Bonus Question: Is the way I safe and organize the desired values common practice? Or are there language constructs in php that I am not aware of?

 <?php

    class nodeStruct {
        /* --- From table *_v, column >i_info< --- */
        public $nodeID = array();   /* The topological ID of the unit within the tree. */
        public $unitLabel;          /* String that describes functionality. */

        /* --- From table *_v, column >nr< --- */
        public $unitID;             /* The actual ID of the unit. */
        public $unitColumn;         /* The column with the measured values for the unit. */
        public $unitName;           /* The name of the unit. */

        /* --- From table *_v, column >a_csv< --- */
        public $unitUnit;           /* Unit that is measured. */

        /* --- From table *_v, column >a_skal< --- */
        public $unitScalFact = 0;   /* Factor for scaling values. */

        /* --- From table *_t, column >z_verbr< --- */
        public $monthSum = array(); /* Sum of the measured values. */
        public $unitSum;            /* Sum of the months. */    

        private $scaleSet = FALSE;

        public function setNodeID($tmpNodeID) {
                $this->nodeID = explode('.', $tmpNodeID);
        }
        public function getNodeID() {
                return "Z  ".implode('.', $this->nodeID);
        }
        public function applyScale() {
                if(!$this->scaleSet) {
                        for($i = 0; $i < 12; $i++) {
                                $this->monthSum[$i] *= (float)$this->unitScalFact;
                        }
                        $this->unitSum *= (float)$this->unitScalFact;
                        $this->scaleSet = TRUE;
                }
        }
        public function getDataLine() {
                $nameUnit = $this->unitName." [".$this->unitUnit."]";
                $ret =  array($this->getNodeID(), $this->unitLabel, $nameUnit);
                $ret = array_merge($ret, $this->monthSum);
                array_push($ret, $this->unitSum);
                return $ret;
        }
}

function generateCsvReport($nodeList) {
        $tmp = ($anl_name." - jahresverbraeuche ".$ja);
        $csvTitle = array("","","","","",$tmp);
        $csvHeader = array("","","Diverse Zaehler","Jaenner","Februar","Maerz","April","Mai","Juni","Juli","August","September","November","Dezember","SUMME");
        $filename = "Jahresbericht";
        $delimiter = ";";

        header('Content-Type: text/x-csv');
        header('Expires: ' . gmdate('D, d M Y H:i:s') . ' GMT');
        header('Content-Disposition: attachment; filename=' . $filename . '.csv');
        header('Pragma: no-cache');

        $comma .= implode(';', $csvTitle)."\n\r";
        $comma .= implode(';', $csvHeader)."\n\r";
        foreach($nodeList as $nodeEl) {
                $line = $nodeEl->getDataLine();
                $comma .= implode(';', $line)."\r\n";
                //fputcsv($f, $line, $delimiter);
        }

        $comma = iconv("utf-8", "windows-1252", $comma);        
        print $comma;
        fclose($f);
}

//******************** übergabe der parameter ***************************************************************************
    session_start();
    $p = $_GET['p'];
    include 'wv_vaph.php';
    $name = $_GET['name'];
    $gr = $_GET['gr'];
    $max_gr = $_GET['sp']; // max anzahl an gruppen
    $ja = $_GET['ja'];
    $nachkomma = 1;

    $valid_hgt = 0;
    if (($hgt_tb != null) && ($hgt_ger != null) && ($hgt_sp != null) && ($hgt_r[1] != null)) {
            $valid_hgt = 2;
    }

    $databasepointer1 = mysql_connect($DatabaseHost, $DatabaseUser, $DatabasePassword);
    mysql_select_db($Database, $databasepointer1);
    mysql_set_charset("utf8");

//******************** namen der variablen aus headertab ************************************************************       

    if ($t == NULL) { 
        exit("Table is not available!"); 
    }

    $head = $t . "_h";  

    // jetzt holen wir mal die namen aus der headertab - die brauchen wir für die anzeige ...
    $resultpointer3=mysql_query("select * from $head order by $head.nr asc") or die (mysql_error());
    $anz_headerdaten = mysql_num_rows($resultpointer3);
    for($i = 0; $i < mysql_num_rows($resultpointer3); $i++)
    {
        $daten3 = mysql_fetch_object($resultpointer3);
        if ($daten3->nr < 50)
        {
            $gerx_nr = $daten3->ger;
            $spx_nr = $daten3->nr;
            $var_name[$gerx_nr][$spx_nr] = $daten3->name;
        }
        else if($daten3->nr != 101)
        {
            $gerx_nr = $daten3->ger;
            $var_name[$gerx_nr][0] = $daten3->name;
        }
    }
    $vartab = $t . "_v";
    $res = mysql_query("select * from $vartab where i_info like '%1%'") or die (mysql_error());
    $rowcnt = mysql_num_rows($res);

    $nodeList = array();

    /* Fetch all available nodes from the _variables table of the project. */
    for ($i = 1; $i < $rowcnt; $i++) {
            $line = mysql_fetch_object($res);
            $nodeList[$i] = new nodeStruct();
            $nodeList[$i]->setNodeID(strtok($line->i_info, ";"));
            $nodeList[$i]->unitLabel    = strtok(";");
            $nodeList[$i]->unitID       = strtok($line->nr, ";");
            $nodeList[$i]->unitColumn   = strtok(";");
            $nodeList[$i]->unitName     = strtok(";");
            $nodeList[$i]->monthSum     = array_fill(0, 12, 0);
            $nodeList[$i]->unitUnit     = strtok($line->a_csv, ";");
            $nodeList[$i]->unitScalFact = $line->a_skal;
    }

    /* Make a query from the _t table for every fetched node and sum the values. */
    $vartab = $t . "_t";    

    foreach ($nodeList as $nodeEl) {
        /* Get all energysums from months. */
        $monthIndex = 0;
        for ($month = 1; $month < 13; $month++) {
            $res = mysql_query("SELECT * FROM $vartab
                                WHERE ((ger = ($nodeEl->unitID)
                                AND sp = ($nodeEl->unitColumn)
                                AND MONTH(timestamp) = $month AND YEAR(timestamp) = $ja))") or die (mysql_error());

            while($line = mysql_fetch_object($res)) {
                $nodeEl->monthSum[$monthIndex] += (float)$line->z_verbr;
            }
            $nodeEl->monthSum[$monthIndex] = number_format($nodeEl->monthSum[$monthIndex], $nachkomma, ',', '');
            $nodeEl->unitSum += (float)$nodeEl->monthSum[$monthIndex];
            $monthIndex++;
        }
        $nodeEl->unitSum = number_format($nodeEl->unitSum, $nachkomma, ',', '');
        $nodeEl->applyScale();
    }


    generateCsvReport($nodeList);
\$\endgroup\$
  • \$\begingroup\$ Minimal examples don't make good Code Review questions. Please show us your real code with contextual information about what your code is designed to accomplish. Also edit the title to state the purpose of the code (see How to Ask). \$\endgroup\$ – 200_success Jul 6 '16 at 7:51
  • \$\begingroup\$ @200_success: Do you think that this question is more appropriate to Stackoverflow as it is more about HOW to achieve something than about how it WAS achieved? If so I could delete and move the question asap. The real code is too broad as it is old code that gets a little bit of refactoring from me currently. \$\endgroup\$ – Haini Jul 6 '16 at 7:59
  • \$\begingroup\$ I don't think that this would make a good Stack Overflow question either, as there is no specific programming problem to be solved. You could make it a good Code Review question, if you would just post your real code. \$\endgroup\$ – 200_success Jul 6 '16 at 8:09
  • 1
    \$\begingroup\$ @200_success: Thanks for the feedback, I have edited the question to hopefully be more conformant to the standards. \$\endgroup\$ – Haini Jul 6 '16 at 8:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.