rosera / pet-theory

Pet Theory (Qwiklabs Quest)
GNU General Public License v3.0
98 stars 95 forks source link

bug (lab01/importTestData.js): syntax error at line 11-25 #64

Open KemingHe opened 4 months ago

KemingHe commented 4 months ago

Bug Report and Fix PR

@rosera

Description

Syntax error in lab01/importTestData.js, line 11-25. Declared err used e. In addition, e is ambiguous as it's used for both csv parsing as well as writing to db. See below.

Location

async function importCsv(csvFilename) {
  const parser = csv.parse({ columns: true, delimiter: ',' }, async function (err, records) {
    if (e) {
      console.error('Error parsing CSV:', e);
      return;
    }
    try {
      console.log(`Call write to Firestore`);
      await writeToDatabase(records);
      console.log(`Wrote ${records.length} records`);
    } catch (e) {
      console.error(e);
      process.exit(1);
    }
  }); 

  await fs.createReadStream(csvFilename).pipe(parser);
}

Proposed Solution

See open PR (number to be added):

  1. Fixed syntax, using pErr for parsing error, and ioErr for database error.
  2. Refactored forEach into for...of for best ES6+ practices.
  3. Added descriptive comments, updated solutions version of importTestData.js as well.
  4. Tested and passed my lab with it.

Solution Code

async function importCsv(csvFilename) {
  const parser = csv.parse(
    { columns: true, delimiter: ',' }, 
    async function (pErr, records) {
      // Gracefully handle parsing errors.
      if (pErr) {
        console.error('Error parsing CSV:', pErr);
        return;
      }

      try {
        console.log(`Call write to Firestore`);
        await writeToDatabase(records);
        console.log(`Wrote ${records.length} records`);

      // Gracefully handle database errors.
      } catch (ioErr) {
        console.error(ioErr);
        process.exit(1);
      }
    }
  ); 

  await fs.createReadStream(csvFilename).pipe(parser);
}
KemingHe commented 4 months ago

Linked PR is #65