Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access lines based on column name instead of its position #19

Open
Nicolas-Bouteille opened this issue Oct 28, 2020 · 2 comments
Open

Comments

@Nicolas-Bouteille
Copy link

Accessing lines by their column name ($line['first_name']) is better than accessing it with its position index ($line[0]) IMHO.
First, it is safer because if you ever make a mistake with the position you might copy the wrong data in the wrong field. That can lead to huge problems and you will need to delete all and import again no matter what. When you use the column name, worst case scenario the column is misspelled so data will be blank or you can stop execution right from the first line.
It is also much more convenient since it allows us to be able to add a new column later on the road in our CSV file and not necessarily place it at the end. We can insert the new column where it makes the most sense without having to rewrite and retest all the import code.
It also makes the code more readable, understandable, easier to maintain...
Here's what I did to pass the line as an associative array with the header key => line value:

$lines = [];
$is_headers_line = TRUE;
$headers = [];
while ($line = fgetcsv($handle, 4096, ';')) {
  if ($is_headers_line) {
    $headers = $line;
    $is_headers_line = FALSE;
  }
  else {
    $lines[] = $line;
  }
}
$nb_lines = count($lines);
foreach ($lines as $line) {
  $line_assoc = array_combine($headers, $line);
  // Use base64_encode to ensure we don't overload the batch
  // processor by stuffing complex objects into it.
  $batch['operations'][] = [
    '\Drupal\csvimport\Batch\CsvImportBatch::csvimportImportLine',
    [array_map('base64_encode', $line_assoc), $csvupload, $nb_lines],
  ];
}
@xurizaemon
Copy link
Owner

Hi @Nicolas-Bouteille & thanks for the neatly organised suggestions (#13 #14 #15 #16 #17 #18 #19)!

If you want to propose any of these as changes for inclusion in the module by opening pull requests, I'm happy to look at merging them. Some (like this) might not suit a PR (an example module like this won't know in advance the column names the person using it needs), but might suit a change to the module's README or docs otherwise.

Hope the code has been useful to you and really appreciate you engaging with it so proactively!

@Nicolas-Bouteille
Copy link
Author

Thank you for your post, indeed your module has been very precious to me. I'll try to follow your example and find some time to push my suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants