1
\$\begingroup\$

I'm currently enrolled in a node.js course. The first app we have done in the course was a terminal notebook. It appends objects to a json-file.

Example call on the terminal:

node notebook.js add Second "Aenean commodo ligula eget dolor."

Example structure json-file:

[{
  "title": "First",
  "timestamp": 1481534161237,
  "body": "Lorem ipsum dolor sit amet, consectetuer adipiscing elit."
}, {
  "title": "Second",
  "timestamp": 1481534192437,
  "body": "Aenean commodo ligula eget dolor."
}]

Then the objects can be displayed, listed or removed.

The basic structure was done together with the trainer. Afterward I added the validation part and an additional property "timestamp". "timestamp" is mainly for listing the notes sorted in ascending order. Furthermore I wrote a function for creating a formatted date (based on the timestamp).

notebook.js (the "main" file):

    // ---- Assignments -----------
    const fs = require('fs');
    const notes = require('./notes.js');

    var args = process.argv;
    var errorReport = "\nSomething has gone wrong.";
    var maxLengthTitle = 150;
    var maxLengthBody = 1000;

    var command = args[2];
    var title = args[3];
    var body = args[4];

    // ---- Validation -----------
    if (['add', 'list', 'read', 'remove'].indexOf(command) === -1) {
       errorReport += `\nParam 1: Expected "add" or "list" or "read" or "remove".
       "${command}" found.`;
    }

    if (command) {
        if (!title && command !== 'list') {
           command = "";
           errorReport += `\nParam 2: Expected string. "undefined" found.`;
        } else if (title && title.length > maxLengthTitle) {
          command = "";
          errorReport +=
      `\nParam 2: Maximal length of title is ${maxLengthTitle} chars.`;
    }
    }

    if (!body || typeof body !== 'string') {
      body = '-';
    } else if (body.length > maxLengthBody) {
        command = "";
        errorReport += `\nParam 3: Length of given second parameter is ${body.length}
        Maximal valid length is ${maxLengthBody} chars.`;
    }
    // -------------------------------------------

    console.log('\n ----- NOTEBOOK ----- ');

    // ---- Reacting to the user input -----------
    if (command === 'add') {
       var note = notes.addNote(title, body);

       if (note) {
          console.log(`Note '${title}' has been added.`)
       } else {
          console.log(`Adding note has failed.'`)
       }
    } else if (command === 'list') {
       var allNotes = notes.getAll();

       allNotes.sort((a, b) => {
          return a - b;
       });

       for (let i = 0; i < allNotes.length; i++) {
          console.log('\n' + notes
           .createFormattedDate(allNotes[i]['timestamp']) + '\n' + allNotes[i]['title'] + '\n' + allNotes[i]['body']);
       }
    } else if (command === 'read') {
       var title = notes.readNote(title);

       console.log(`Title is : ${title} !`);
    } else if (command === 'remove') {
       notes.removeNote(title);

       console.log(`Note '${title}' has been removed.`)
    } else {
       console.log(errorReport);
    }
    // ----------------------------------------

    console.log('\n -------------------- \n');

notes.js (containing the module with the actual functions):

const fs = require('fs');

var fetchNotes = () => {
  try {
    var notesString = fs.readFileSync('notes-data.json');
    return JSON.parse(notesString);
  } catch (e) {
    return [];
  }
};

var saveNotes = (notes) => {
  fs.writeFileSync('notes-data.json', JSON.stringify(notes));
}

var addNote = (title, body) => {
  var notes = fetchNotes();
  var note = {
    title: title.trim(),
    timestamp: Date.now(),
    body: body.trim()
  };

  notes.push(note);
  saveNotes(notes);

  return note;
}

var readNote = (title) => {
  var notes = fetchNotes();

  var ret = notes.filter((note) => {
    return note.title === title;
  });

  return ret[0] ? ret[0].title + '\n' + ret[0].body : '';
}

var getAll = () => {
  return fetchNotes();
}

var getNote = (title) => {
  console.log('Get single node: ', title);
}

var removeNote = (title) => {
  var notes = fetchNotes();
  var newNotes = notes.filter((note) => note.title !== title);

  fs.writeFileSync('notes-data.json', JSON.stringify(notes));
}

var createFormattedDate = (timestamp) => {
  var date = new Date(timestamp);
  var ret = ('0' + date.getDate()).slice(-2) + '.' + ('0' + (date.getMonth() + 1)).slice(-2) + '.' + date.getFullYear() + ', ' + ('0' + date.getHours()).slice(-2) + ':' + ('0' + date.getMinutes()).slice(-2) + ':' + ('0' + date.getSeconds()).slice(-2);

  return ret;
}

module.exports = {
  addNote,
  getAll,
  getNote,
  removeNote,
  readNote,
  createFormattedDate
}

It all works but "notebook.js" seems rather messy to me. How could it be better structured? What other improvements could be done?

I guess there a tasks which could be accomplished less awkward then I have done, especially concerning the formatted-date function.

Was it a good choice to use a date value for sorting the records? Or would another data type be more appropriate??

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$
  • One of the things a course should teach you is to take care when you decide what should be const, and what should not be const. Example of what ought to be const are maxLengthTitle and maxLengthBody
  • Moving process.argv to args is sugar, it makes the lines 10 to 12 easier to read. However, in this case, I would have foregone the move and just use process.argv in lines 10 to 12. The code would be 1 line shorter, and not harder to read.
  • You would get extra points if you could derive 'Expected "add" or "list" or "read" or "remove".' from the array you declared just prior to that with ['add', 'list', 'read', 'remove']
  • I would not clear command if there is an error, I would just make the first if after --reacting to the user input -- if( errorReport ){ and go from there. I would also future proof that variable and call it output or feedback
  • I am wondering about typeof body !== 'string', what case are you covering there?
  • Run your code thru jshint.com, you have a few missing semicolons, some accessors that could use dot notation, and an unused library in notebook.js
  • Read up on model view controller (MVC), your controller code (especially for command 'list', is doing far too much, in essence your code should read

    if(command == "list"){
      var allNotes = notes.getAll();
      console.log( formatNotes( allNotes );
     }
    

    and then drop the mic

  • Finally, this drives me nuts:

    var getAll = () => {
      return fetchNotes();
    }
    

    Fat arrow syntax is meant for inline functions, please just use

    function getAll(){
      ..
    }
    
\$\endgroup\$
3
  • \$\begingroup\$ Awesome answer. Helped me to understand arrow functions. As inline function it makes perfect sense that they always got the this of the current context. Thanks a lot. \$\endgroup\$ Dec 13, 2016 at 6:29
  • \$\begingroup\$ The idea with typeof body !== 'string' : In case someone has given another data type (e.g. number) then it wouldn't be empty. So it continuees with the next if-else. It would try to access the length property. This would cause an exception because most data types does not have that property. \$\endgroup\$ Dec 14, 2016 at 8:21
  • \$\begingroup\$ for the last bullet, how about const getAll = fetchNotes? \$\endgroup\$
    – nabn
    Dec 17, 2016 at 5:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.