Hacker News Reader

Hacker News

Comment by sfn42 | Hacker News Reader

Parent Full thread

sfn42

I would have written each statement on its own line:

var users = db.getUsers();

var expiredUsers = getExpiredUsers(users, Date.now());

var expiryEmails = generateExpiryEmails(expiredUsers);

email.bulkSend(expiryEmails);

This is not only much easier to read, it's also easier to follow in a stack trace and it's easier to debug. IMO it's just flat out better unless you're code golfing.

I'd also combine the first two steps by creating a DB query that just gets expired users directly rather than fetching all users and filtering them in memory:

expiredUsers = db.getExpiredUsers(Date.now());

Now I'm probably mostly getting zero or a few users rather than thousands or millions.

View on HN

Yeah. I did not mention what I would do, but what you wrote is pretty much what I prefer. I guess nobody likes it these days because it is old procedural style.

There's nothing procedural about binding return values to variables, so long as you aren't mutating them. Every functional language lets you do that. That's `let ... in` in Haskell.

(author here)

This is actually closer to the way the first draft of this article was written. Unfortunately, some readability was lost to make it fit on a single page. 100% agree that a statement like this is harder to reason about and should be broken up into multiple statements or chained to be on multiple lines.

Took me a bit of scrolling to find this. I believe most of the other folks are functional devs or something. The 5 functions on a single line wouldn't pass the code review in most .net/java shops.

The rule I was raised with was: you write the code once and someone in the future (even your future self) reads it 100 times.

You win nothing by having it all smashed together like sardines in a tin. Make it work, make it efficient and make it readable.

Glad to see this. This style seems like it’s out of vogue now, but I find it much, much easier to reason about.

I agree because it reads as it will process in the direction I normally read. But I do think one of the benefits of the function approach is that the scope isn't cluttered with staging variables.

For these reasons one of the things I like to do in Swift is set up a function called ƒ that takes a single closure parameter. This is super minimal because Swift doesn't require parenthesis for the trailing closure. It allows me to do the above inline without cluttering the scope while also not increasing the amount of redirection using discrete function declarations would cause.

The above then just looks like this:

  ƒ { 
    var users = db.getUsers();
    var expiredUsers = getExpiredUsers(users, Date.now());
    var expiryEmails = generateExpiryEmails(expiredUsers);\
    email.bulkSend(expiryEmails);
  }

But then you are creating references with larger then needed "reachability".

I don't see a problem with that. This code would typically be inside it's own function anyway, but regardless I think your nitpick is less important than the readability benefit.

Reply on news.ycombinator.com