Search code examples
phpjsonclassfile-get-contents

PHP: Is it a bad practice to use file_get_contents() in a class __constructor?


I have a class that makes an economic calendar out of a json string. The only problem is that I don't know if I should use file_get_contents()(to get the data from an api) inside my class __constructor() or I should just pass the json string to the __constructor from my try{...}catch{...} block?

Which practice is better and why?

Here is my class(EconomicCalendar.php) so far:

class EconomicCalendar{

    private $_data,
            $_calendar = [];

    public function __construct($url){
        $this->_data = json_decode(file_get_contents($url));
    }

    private function make_economic_calendar(){
        foreach($this->_data->events as $e){
            $arr[$e->date][] = [
                'title' => $e->title,
                'date' => $e->date
            ];
        } 

        if(is_array($arr) && count($arr) >= 1){
            return (object)$arr;
        } else{
            throw new Exception('EC was not created');
        }
    }

    public function get_calendar(){
        $this->_calendar = $this->make_economic_calendar();
        return $this->_calendar;
    }

}

Here is the code(ec.php) that outputs the calendar:

spl_autoload_register(function($class){
    require_once dirname(__FILE__) . DIRECTORY_SEPARATOR . $class . '.php';
});

try {
    $c = new EconomicCalendar('https://api.example.com/ec?token={MY_TOKEN}');
    $economic_calendar = $c->get_e_list(); 
} catch (Exception $e) {
    exit($e->getMessage());
}

Thank you!


Solution

  • Almost always is better to make IO operation as late (or as little) as possible. So I recommend you to use "named constructor" if you want initialize with data

    class EconomicCalendar {
    
        ...
    
        public function __construct($data){
            $this->_data = $data;
        }
    
        ...
    
        public static function fromUrl($url){
            return new self(json_decode(file_get_contents($url)));
        }
    
    }
    

    And usage:

    $instance = EconomicCalendar::fromUrl('https://api.example.com/ec?token={MY_TOKEN}');
    

    Moving IO and decoding to dedicated function is closer to single responsibility principle (IO at static, logic at class instance).