Hacker News new | past | comments | ask | show | jobs | submit login

Yes it doesn't handle the error case.

And if you replace the if(err) lines with a log(...) function it doesn't reduce them to one place. It makes you repeat the log(...) function everywhere. And you'd still need the if statement to handle the control flow.

Simple code is great, but not handling errors doesn't cut it for non throwaway applications.




You assume a lot.

We can refactor another round:

  function logFsResult( type, err ){
  	var msg= '';
  	switch ( type ) {
  		case 'append': msg= ( err ) ? 'Failed to update file' : 'Successfully updated file';
  			break;
  		case 'write': msg= ( err ) ? 'Failed to write file' : 'Successfully created file';
  			break;
  		default: msg= 'logFsResult error, missing or invalid first argument: '+ (type || '')
  	}
  	( err ) ? console.err( msg ) : console.log( msg );
  }

  function handleFile( filename, fileExists ) {
  	const timestamp = new Date().toISOString();
  	( fileExists )
  		?	fs.appendFile( filename, `Updated file on ${timestamp}\n`, logFsResult.bind(null, 'append') )
  		:	fs.writeFile( filename, `Created file on ${timestamp}\n`, logFsResult.bind(null, 'write') );
  }


Whenever you program in ECMAScript2016("Javascript"), you should take advantage of its features. Right now you're coding pretty much using the same way one would do it in C. Take advantage that functions are first-class in ES2016. Take advantage of JSON.

For example you could do something like this (sorry, don't have time to open my IDE to try the code i'm going to write):

    logFsResult = (type, err) => {
        map_action = {
          "append": { 
            True: "Succesfully updated file.",
            False: "Failed to update file."
          },
          "write": {
            True: "Successfuly created file.",
            False: "Failed to write file."
          }
        }

        message = map_action[type][(err != null)] // obtain the message
          method = (err)? console.err : console.log // choose logger
          method(err) //invoke the correct logger with the message

    }
This is an easier-to-mantain code. It separates the messages to use, from the logic. It allows you to easily configure/change the messages to log, and the method (function) for logging the error, without having to touch the decision logic. On your original example, the logic of the code was mingled with the error messages.

You could say this example was more "idiomatic" ES2016.


> Whenever you program in ECMAScript2016("Javascript"), you should take advantage of its features.

That's a trap, I should rather make my code as readable, scalable and bug free as possible regardless of ESxxx.

I can refactor in 10 other ways (different styles) coming to the same result, but that's not what my point was about. Using promises and so is just taste or preference, if you like it you use can it, if not do without. I've seen amazing spaghetti with promises and callbacks as well.

Easy to nitpick btw:

you compare err != null?? besides not using strict mode, what should err be? a String? So, what will happen if err is undefined or an empty String?

Then you call logFsResult with err while it is not used.. Did you even consider what happens if the value of type is not available in map_action? I'll be the end of the feast!

last one: True and False as an object key are not Booleans, so if you have your IDE up and running, the code will fail.

Now, you can try to solve this with promises, just as you can try brush your teeth with a broomstick.


> you compare err != null?? besides not using strict mode, what should err be? a String? So, what will happen if err is undefined or an empty String?

YOU are the one who made that comparison, not me. You originally wrote the following line:

   ( err ) ? console.err( msg ) : console.log( msg );
What do you think the "?" operator does with "err"?

> Then you call logFsResult with err while it is not used..

It seems you don't understand the code. I'm not calling "logFsResult", i am defining a function called logFsResult. You also did the same, you defined logFsResult to receive the "err" parameter.

  function logFsResult( type, err ){
  	var msg= '';
  	switch ( type ) {
> That's a trap, I should rather make my code as readable, scalable and bug free as possible regardless of ESxxx.

ES6 allows you to write more readable code than ES5. Take a look at the features.


That's still not error handling, that's error logging. Try writing code that depends on the success of the previous operation to perform another operation and you'll quickly find yourself in the callback soup.


Honestly that looks hideous.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: