views:

237

answers:

2

I've attempted to write a PHP class that gets the uptime of my Linux machine. It will get the uptime correctly, but I have an if statement that determines whether or not the load average is "high" or not and set a warning code, and it doesn't seem to be working (it stays at 0).

I've included all of the code here from the class (50 or so lines) because I didn't know what I could take out but still provide some info about what's going wrong here.

<?php
class loadavg {

    private $divisor, $status;

    public function __construct($set_divisor = 1){
        $this->divisor = $set_divisor;
    }

    public function __toString(){
        return $this->load(1).', '.$this->load(5).', '.$this->load(15)."\n";
    }

    public function load($time = 1){
        $loadfile = shell_exec('cat /proc/loadavg');
        $split = preg_split('/ /', $loadfile);

        if ($split[1] > (2 * $this->divisor)){
            $this->status = 3;
        } else if ($split[1] > $this->divisor){
            $this->status = 2;
        } else {
            $this->status = 1;
        }

        switch($time){
            case 1:
                return $split[0];
            case 5:
                return $split[1];
            case 15:
            return $split[2];
        }
    }

    public function status_name(){
        switch ($this->status){
            case 3:
                return 'critical';
            case 2:
                return 'warn';
            case 1:
                return 'ok';
            case 0:
                return 'error';
        }
    }
}
?>
A: 

Your code appears to work fine, though the design makes its use less intuitive that it could be. I put your class definition in a file, added the following lines to the bottom, and it prints 'ok' when run.

$load = new loadavg ();
$load->load (5);
echo $load->status_name () . "\n";

The strange thing about the design of your class, to me, is that it does not allow 'loading' at initialization, but only calls load automatically when you try to print the loadavg object. If you want to be able to access status_name () without first explicitly calling load (), you can just call load in __construct (perhaps with a sensible default that can be overridden just like the divisor...)

grossvogel
A: 
<?
class loadavg {

    private $divisor, $status;

    public function __construct($set_divisor = 1){
        $this->divisor = $set_divisor;
    }

    public function __toString(){
        return $this->load(1).', '.$this->load(5).', '.$this->load(15)."\n";
    }

    public function load($time = 1){
        //$loadfile = shell_exec('cat /proc/loadavg');
        $loadfile = '1.5 1.5 1.5';
        $split = preg_split('/ /', $loadfile);

        if ($split[1] > (2 * $this->divisor)){
            $this->status = 3;
        } else if ($split[1] > $this->divisor){
            $this->status = 2;
        } else {
            $this->status = 1;
        }

        switch($time){
            case 1:
                return $split[0];
            case 5:
                return $split[1];
            case 15:
            return $split[2];
        }
    }

    public function status_name(){
        switch ($this->status){
            case 3:
                return 'critical';
            case 2:
                return 'warn';
            case 1:
                return 'ok';
            case 0:
                return 'error';
        }
    }
}

$la = new loadavg();
print($la);
print($la->status_name()); 

I just executed this, and got the output:

1.5, 1.5, 1.5
warn

Isn't this what is expected? If I take out the print($la) statement, the output is this:

error

this is because the load() function is never called to set the status. If you want to have the status printed out correctly, you have to run load() first. I assume you'd want to add the time parameter to status_name and call load before returning, like this:

public function status_name($time =1){
    $this->load($time);
    switch ($this->status){
        case 3:
            return 'critical';
        case 2:
            return 'warn';
        case 1:
            return 'ok';
        case 0:
            return 'error';
    }
}

which will now enable you to do this:

$la = new loadavg();
print($la->status_name(5)); 
Mike Sherov
I see, load() needs to be run within status_name() for it to work. Thanks.
Peeps