Tools for transferring data from an API to a database

Question

I explain everything on my current code but this question is more general "HOW TO" work correctly with this in this case but even next time.

I have 4 methods in my class, each does something but in general this code just takes data from some API and insert it to another database in the right format:

// Getting data from API
public function getData()
{
    $url = manufacturerEndpoint;
    $data = array('token' => apiToken);

    // use key 'http' even if you send the request to https://...
    $options = array(
        'http' => array(
            'header'  => "Content-type: application/x-www-form-urlencoded\r\n",
            'method'  => 'POST',
            'content' => http_build_query($data),
        ),
    );
    $context  = stream_context_create($options);
    $result = file_get_contents($url, false, $context);

    return $result;
}  

// Truncate oild data in new table
private function truncate() {                  
    try{
        $con = new PDO( DB_HOST, DB_USER, DB_PASS ); 
        $con->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
        $sql = "truncate r_vehicle_manufacturer CASCADE";
        $stmt = $con->prepare( $sql );
        $stmt->execute();                    
        }catch(PDOExeption $e){
            echo $e->getMessage();
            echo $con->errorInfo();
        }                            
} 

// Inset new data to database
public function insertToDb($code, $name)
{       
    try {                                       
        $con = new PDO( DB_HOST, DB_USER, DB_PASS );            
        $con->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );                        
        $sql = "INSERT INTO r_vehicle_manufacturer(code, name, country_code) VALUES(:code, :name, :country_code)";
        $stmt = $con->prepare( $sql );
        $stmt->bindValue( 'code', $code, PDO::PARAM_STR );                
        $stmt->bindValue( 'name', $name, PDO::PARAM_STR );                           
        $stmt->bindValue( 'country_code', 'CZE', PDO::PARAM_STR );
        $stmt->execute();
        }
    catch( PDOException $e ) {
            echo $e->getMessage();
        }       
}
// Last method which basicly just call others:
        public function workWithData()
    {
        // Truncate data from last month
        $this->truncate();
        $json = $this->getData();
        $json_output = json_decode($json);          
        foreach ( $json_output as $data )
        {
            $this->insertToDb($data->AX_CODE, $data->NAME);
        }
    }

I'm not sure if this is correct way but until now it seems right for me. But now I just have to change $url (call this method with multiple URL base on user INPUT) sometimes but it's not easy to edit if I don't want to pass the same argument to 2 methods.

My question is how should I correctly design methods to avoid as many changes as I can while respecting the fact I will have to change something in feature?

Answer 1

Styling and readability

Your indentation and use of whitespace is all over the place. Make sure to use consistent indentation. If you decide to add whitespace somewhere, use the same rules for whitespace elsewhere. In particular, pay attention to where you place your braces. Sometimes you put them on a new line, sometimes you do not. Sometimes you put the closing brace before a corresponding catch, sometimes you place a newline. Sometimes you indent it, sometimes you do not. Choose a style, and use it for all function definitions, for-loops, try-catch blocks etc.

Many connections

For each action you make a separate connection. You can improve the performance of your script by creating one connection, and make every query use that connection instead.

General bugs

file_get_contents(..) returns FALSE on failure. The documentation is not concise about this, but I assume that a timeout will cause it to fail. Your script should do something sensible in this case instead of trying to decode it as json.

json_decode(..) returns NULL if the result cannot be decoded properly (e.g. someone replaced the resource to "Use v.77.2 you silly goose") or the recursion limit is exceeded. You do not check for this condition.

If one of the inserts fails, do you really want to just ignore that insert and continue inserting? Do you really not want to retry it at the very least (in case of a failed connection for example).

Maintainability

You are using magic values in some places. The table name r_vehicle_manufacturer is one example of that. $url = manufacturerEndpoint; looks weird to me and might be an other one if that is your way of anonymizing the code.

This seems to be an (incomplete) class definition. I feel that the function to retrieve data from an api and the functions to interact with the database should not be in the same class.