I'm going through the CoffeeScript book by Trevor Burnham, and I was curious as to what the best style is for a function I was writing.

In the book the author writes the function like so:

strToCoordinates = (input) ->
    halves = input.split(',')
    if halves.length is 2
        x = parseFloat halves[0]
        y = parseFloat halves[1]
        if !isInteger(x) or !isInteger(y)
            console.log "Each coordinate must be an integer."
        else if not inRange x - 1, y - 1
            console.log "Each coordinate must be between 1 and #{GRID_SIZE}."
        else
            {x, y}
    else
        console.log 'Input must be of the form `x, y`.'

I wrote my function like so:

strToCoordinates = (input) ->
    halves = input.split(',')
    if halves.length isnt 2
        console.log 'Please enter coordinates in format: x,y'; return
    [x, y] = (parseFloat s for s in halves)
    unless isInteger(x) and isInteger(y)
        console.log 'The coordinates must be integers.'; return
    if not inRange(x, y)
        console.log "Coordinates must be between 1 and #{GRID_SIZE}."; return
    {x, y}

Is my use of just if statements and using return to stop the flow if the check fails okay style?

I like the fact that with this style the error messages are right after the check unlike with if/else, and also you don't have to have large sections indented.

share|improve this question

4 Answers

Well I don't now coffeescript, but guess I can read it anyway :D

I think both are good and readable.
I don't know if you can add empty lines to the code. If you can:

strToCoordinates = (input) ->
    halves = input.split(',')
    if halves.length isnt 2
        console.log 'Please enter coordinates in format: x,y'; 
        return

    [x, y] = (parseFloat s for s in halves)
    unless isInteger(x) and isInteger(y)
        console.log 'The coordinates must be integers.'; 
        return

    if not inRange(x, y)
        console.log "Coordinates must be between 1 and #{GRID_SIZE}."; 
        return

    {x, y}

But this really is a minor arguable tweak (some will like it, some don't, I do).
Another point would be the "overuse" of returns. Functions are easier to read, if they do have lss return-statements. But Again, this usually applies to larger functions only.

share|improve this answer

I don't really like hiding the returns off the end of the line, so putting them on separate lines (or reverting to the original) would be preferable.

However, I'd also consider actually using postfix if. This would make the code read like this

strToCoordinates = (input) ->
    halves = input.split(',')
    return console.log 'Please enter coordinates in format: [x,y]' if halves.length isnt 2
    [x, y] = (parseFloat s for s in halves)
    return console.log 'The coordinates must be integers.' unless isInteger(x) and isInteger(y)
    return console.log "Coordinates must be between 1 and #{GRID_SIZE}." unless inRange(x, y)
    {x, y}

Under certain circumstances, postfix if can be buried at the end of the line. The presence of code straight after a return draws the eye to the conditions in this case. This would be my favoured way of writing the function as not only is it concise, it would be very easy to extend to extra conditions.

share|improve this answer

I would agree with the author's style just because it's not the absolute case.

If the author wants to build on the example and introduce more cases like a Z coordinate it's an else if statement, but your function you would have to have another inner if statement to handle there being three coordinates.

 if isnt 2 and 3 
      ...
      if 2 
        ...
      elseif 3

In this example you wouldn't be adding to it anyway since it's an excerise, but if it's code you want to use in a project then there's no telling what new requirements you might add in.

share|improve this answer
strToCoordinates = (input) ->
    [x, y] = (+z for z in (halves = input.split ',') if halves.length is 2)
    unless x isnt undefined and y isnt undefined then (console.log 'Input must be of the form `x, y`.'; return)
    unless x>>0 is x then (console.log "Each coordinate must be an integer."; return)
    unless inRange x - 1, y - 1 then (console.log "Each coordinate must be between 1 and #{GRID_SIZE}."; return)
    {x, y}

Just personally.

share|improve this answer
Note: Great thing about coffee script, or a bad thing, is that it's essentially style-less. – Not a Name Feb 27 '12 at 19:07

Your Answer

 
or
required, but never shown
discard

By posting your answer, you agree to the privacy policy and terms of service.

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