2
\$\begingroup\$

I need to make a simple CSV->MySQL script in OOP and I was wondering if there's anything I am doing wrong in a object-oriented sense.

I have a file name db.php where I connect to MySQL via MySQLi and then I have another class where I load a csv file into MySQL like this:

class csv extends Database { public $file = null; function __construct() { if(count($_SERVER["argv"]) > 1){ $this->file = $_SERVER["argv"][1]; }else{ $this->file = "stock.csv"; } $this->db = $db; } function saveCSVtoDB(){ //opens the file $fp = fopen($this->file, "r"); $db = Database::getInstance(); $mysqli = $db->getConnection(); $result = $mysqli->query("LOAD DATA LOCAL INFILE 'file.csv' IGNORE INTO TABLE products CHARACTER SET UTF8 FIELDS TERMINATED BY ',' LINES TERMINATED BY '\n' IGNORE 1 LINES (code, name, desc, stock, price, @avai) SET available = IF(@avai LIKE '%yes%', 1, 0) "); if($result){ echo "success"; }else{ die("error"); } } } 
\$\endgroup\$
0

    2 Answers 2

    4
    \$\begingroup\$

    So I would recommend to not extend the CSV parser from the Database class. Instead The CSV parser should get the Database per dependency injection through the constructor. Also I would not recommend to access gobal vars like $_SERVER in a class. Here an example how you could do that:

    class CSVParser { protected $file; protected $database; public function __construct($file, Database $db) { $this->file = $file; $this->db = $db; } public function saveCSVtoDB() { // @TODO import CSV to DB } } 

    And you could use the class with:

    $file = isset($_SERVER["argv"][1]) ? $_SERVER["argv"][1] : "stock.csv"; $parser = new CSVParser($file, Database::getInstance()); $parser->saveCSVtoDB(); 
    \$\endgroup\$
      0
      \$\begingroup\$

      Hard-coded Values

      Your code contains a lot of hard coded values, from the file name to the columns you expect it to contain and even some custom logic if the form on the if statement inside the query.

      Should you want to create a more generic, reusable solution, all these inputs should be customizable.

      Importing CSVs like that has constraints

      When you specify a filename to the relational database so that it imports data, the database server searches for that file relative to it's filesystem. It's quite common that the database server runs on a different machine than the one your web server runs under. This needs to be taken into consideration.

      Given this information, you open the file in PHP for no apparent reason.

      \$\endgroup\$

        Start asking to get answers

        Find the answer to your question by asking.

        Ask question

        Explore related questions

        See similar questions with these tags.