Page 2 of 3
Re: Using PDO and running into problems
Posted: Tue May 01, 2012 8:09 pm
by Pavilion
Celauran wrote:Try calling trim() on the email addresses before validating them. '
duck@disney.com' is not a valid address (note the leading space).
Celauran - THANK YOU!!!!! Your advice was right on. I have no idea what was off with my "valid" email addresses. Nothing visible to the eye. But, once I ran the variable through a trim, everything fell into place. Plus I got a bonus out of the process.
One of the things I needed to do for "clean up" of data presented by users was to get rid of top rows where users entered titles into the .csv file. But.. using this process the title of the email column is not considered valid and so it is not allowed to proceed into the PDO block.
I've one other thing that needs to addressed to clean up user files before INSERT. If a user uploads a .csv file with quotation marks around all the values, how do I strip those quotation marks out of the string BEFORE extracting values and assigning them to variables in the foreach block?
Also, under the heading of, "never trust a user"... are there any other clean up processes I'm missing, besides those already discussed?
Thanks so much Celauran - your advice is really appreciated.
Pavilion
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 6:01 am
by Celauran
Pavilion wrote:If a user uploads a .csv file with quotation marks around all the values, how do I strip those quotation marks out of the string BEFORE extracting values and assigning them to variables in the foreach block?
Why before? You can just call a second trim operation with quotes as the optional second parameter.
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 6:30 am
by Pavilion
Why before? You can just call a second trim operation with quotes as the optional second parameter.
If I use a second trim function after extracting each column, wouldn't I have to run the trim function on EVERY column?
Code: Select all
$col_0=$inner_array[0];
$col_1=$inner_array[1];
$col_2=$inner_array[2];
$col_3=$inner_array[3];
$col_4=$inner_array[4];
$col_5=$inner_array[5];
$col_6=$inner_array[6];
A trim function for each of the 7 columns?
Wouldn't it be more efficient to strip quotations from the whole row string BEFORE extracting column data?
Pavilion
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 6:50 am
by Pavilion
Celauran:
Here is my solution to the problem. By using
str_replace it is possible to strip all quotation marks from the array BEFORE extracting individual column values. Following is the
foreach block as it stands now.
Code: Select all
foreach(split($lineseparator,$csvcontent) as $line) {
$outer_array = (str_replace(chr(34),"", $line));
$inner_array = explode($fieldseparator,$outer_array);
$col_0=$inner_array[0];
$col_1=$inner_array[1];
$col_2=$inner_array[2];
$col_3=$inner_array[3];
$col_4=$inner_array[4];
$col_5=$inner_array[5];
$col_6=$inner_array[6];
$col_7=$userid;
$email = trim($col_6);
$e_mail = filter_var($email, FILTER_VALIDATE_EMAIL);
if ($e_mail)
{ /// Run PDO and INSERT queries
Is there a more efficient way to handle the issue? This approach works for user uploaded files with :
- quotation marks around column values
- invalid email addresses
- files with first rows as column headings
- clean files from educated users.
Since this is my first attempt at such procedures, your input as to the efficiency of my script is valued. For instance, experience tells me that if code is not written "right" in the first place it can cause major headaches down the road. This is why I am sincerely asking you for input. Am I setting myself up for problems down the road?
Thanks again - Pavilioini
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 8:01 am
by Celauran
The block presented above seems fine the way it is. I'm curious how you're reading in the csv, though. Using
fgetcsv(), you shouldn't need to remove delimiters manually.
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 9:02 am
by Pavilion
Hello Celauran:
The block presented above seems fine the way it is. I'm curious how you're reading in the csv, though. Using fgetcsv(), you shouldn't need to remove delimiters manually.
Yes - I wondered if you'd ask about fgetcsv(). As it happens the script example I decided to use allows for upload of different types of files:
Code: Select all
$mimes = array('application/vnd.ms-excel','application/octet-stream','text/plain','text/csv','text/tsv');
So... I was a bit leery about using
fgetcsv() and am instead using the following block of script to test the file:
.....
Code: Select all
$csvfile = "upload/" . $_SESSION['user_id'] . "_" . $_FILES["file"]["name"];
.....
Code: Select all
$fieldseparator = ",";
$lineseparator = "\n";
// Test file.
if(!file_exists($csvfile)) {
echo "File not found. Make sure you specified the correct path.\n";
exit;
}
$file = fopen($csvfile,"r");
if(!$file) {
echo "Error opening data file.\n";
exit;
}
$size = filesize($csvfile);
if(!$size) {
echo "File is empty.\n";
exit;
}
$csvcontent = fread($file,$size);
fclose($file);
After this block of code, the INSERT statements and foreach blocks run.
Hope this helps, and as before, if I'm setting myself up for headaches further down the road, please do warn me.
Thanks Much:
Pavilion
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 9:20 am
by x_mutatis_mutandis_x
Pavilion wrote:Yes - I wondered if you'd ask about fgetcsv(). As it happens the script example I decided to use allows for upload of different types of files:
Determine your file type and based on it perform appropriate processing, where you can use fgetcsv if it's a csv file:
Code: Select all
function processFile($file) {
switch (pathinfo($file, PATH_INFO_EXTENSION)) {
case 'xls':
case 'xlsx':
return processExcelXls($file);
break;
case 'csv':
return processExcelCsv($file);
break;
//.....
default:
return file_get_contents($file); //or echo error, however you want to handle the default case
}
}
function processExcelXls($filename) {
//Use PHPExcel library (see url below for documentation/api)
}
function processExcelCsv($filename) {
//Use fgetcsv() here
}
//....
In OOP, using classes and abstractions, this is called a "factory design pattern".
Here's the documentation for
PHPExcel
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 10:18 am
by Pavilion
x_mutatis_mutandis_x wrote:Determine your file type and based on it perform appropriate processing, where you can use fgetcsv if it's a csv file:
Code: Select all
function processFile($file) {
switch (pathinfo($file, PATH_INFO_EXTENSION)) {
case 'xls':
case 'xlsx':
return processExcelXls($file);
break;
case 'csv':
return processExcelCsv($file);
break;
//.....
default:
return file_get_contents($file); //or echo error, however you want to handle the default case
}
}
function processExcelXls($filename) {
//Use PHPExcel library (see url below for documentation/api)
}
function processExcelCsv($filename) {
//Use fgetcsv() here
}
//....
In OOP, using classes and abstractions, this is called a "factory design pattern".
OK ... and I am being completely sincere here... what is the advantage of this approach. I've no experience at all with php, javascript, html, css, etc... But.. I do have 20 years experience building classical databases. At the end of the day what is "right" when writing code isn't always about what is "in" or what is "the way everyone else is doing it". When constructing code one has to keep in mind all sorts of things, efficiency, what works for the "long-haul", what creates a user-friendly front-end, what is most "flexible" going forward.
The example script you provide requires a lot more lines, so what is the pay-off? Please understand, I'm not being sarcastic, or trying to push your buttons. I truly want to know the advantages to your approach.
Thanks so much for your patience with my queries.
Pavilion.
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 10:25 am
by Celauran
It allows you to use the right tool for the job rather than using file() and then trying to parse the file on your own, trim and remove enclosing quotes if needed, etc. If you decide to allow additional file types in the future, you can easily add them in to the processFile function, sending them off to custom functions for parsing if/as needed, without having to rework your existing code. Each function has its own very specific purpose and is in no way tied to any other function (except processFile() itself, obviously).
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 10:41 am
by x_mutatis_mutandis_x
Pavilion wrote:When constructing code one has to keep in mind all sorts of things, efficiency, what works for the "long-haul", what creates a user-friendly front-end, what is most "flexible" going forward.
And using efficient design patterns help you do exactly that.
The example script you provide requires a lot more lines, so what is the pay-off?
More lines of code doesn't necessarily mean it's a bad thing. Besides I have suggested using a third party library which will actually reduce your lines of code.
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 10:52 am
by Pavilion
x_mutatis_mutandis_x wrote:Pavilion wrote:When constructing code one has to keep in mind all sorts of things, efficiency, what works for the "long-haul", what creates a user-friendly front-end, what is most "flexible" going forward.
And using efficient design patterns help you do exactly that.
I understand this... I really do. I'm not trying to pick at you. What I'm trying to do is learn.
The example script you provide requires a lot more lines, so what is the pay-off?
More lines of code
doesn't necessarily mean it's a bad thing. Besides I have suggested using a third party library which will actually reduce your lines of code.
Again - we agree
What I'm trying to understand here .. is WHY? Why does your approach have advantages over my attempt (and I use the word "attempt" here because I am learning and so far I don't consider my attempt an approach)? What is the pay-off for more extensive script construction?
Thanks for your patience:
Pavilion
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 11:09 am
by x_mutatis_mutandis_x
The main advantage is that when a user uploads any file, you have a way to determine which file is it, and how to process the file content. I suggested this approach because you were mentioning that your script allows for uploading multiple file types. This way, your code is clean, and easier to maintain (refer to Celauran's post above for details).
You also can have this code in an another php file called "functions_processFile.php" and your main script will look like:
Code: Select all
//.....
$file = "upload/" . $_SESSION['user_id'] . "_" . $_FILES["file"]["name"];
// Test file.
if(!file_exists($file)) {
echo "File not found. Make sure you specified the correct path.\n";
exit;
}
include_once 'functions_processFile.php';
$data = processFile($file);
//.......
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 4:02 pm
by Pavilion
x_mutatis_mutandis_x wrote:The main advantage is that when a user uploads any file, you have a way to determine which file is it, and how to process the file content. I suggested this approach because you were mentioning that your script allows for uploading multiple file types. This way, your code is clean, and easier to maintain (refer to Celauran's post above for details).
You also can have this code in an another php file called "functions_processFile.php" and your main script will look like:
Code: Select all
//.....
$file = "upload/" . $_SESSION['user_id'] . "_" . $_FILES["file"]["name"];
// Test file.
if(!file_exists($file)) {
echo "File not found. Make sure you specified the correct path.\n";
exit;
}
include_once 'functions_processFile.php';
$data = processFile($file);
//.......
OK - you and Celauran have convinced me (I hadn't seen Celauran's earlier post until you pointed it out). So... more things to learn... I'll be working on them tonight.
Thanks so much:
Pavilion
Re: Using PDO and running into problems
Posted: Wed May 02, 2012 4:26 pm
by Christopher
I didn't see it mentioned above, but for security you should use
is_uploaded_file($_FILES["file"]["tmp_name"]) to check the uploaded file and
move_uploaded_file() to move the file to it actual name. Probably in some other code not posted, but I thought I would mention it.
Re: Using PDO and running into problems
Posted: Thu May 03, 2012 7:20 am
by Pavilion
I didn't see it mentioned above, but for security you should use is_uploaded_file($_FILES["file"]["tmp_name"]) to check the uploaded file and move_uploaded_file() to move the file to it actual name. Probably in some other code not posted, but I thought I would mention it.
Thanks Christopher:
Move_uploaded_file() had already been incorporated. But, your advice on
is_uploaded_file() is well taken and I've just finished including that in the script.
Now - to work on the script suggestions from others...
Pavilion