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");
 }
 }
}
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Apr 24, 2017 at 17:31
\$\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();
answered Apr 24, 2017 at 17:58
\$\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.

answered Apr 24, 2017 at 18:50
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.