Hacker News Reader

Comment by baddox | Hacker News Reader

I agree with you. I think it becomes pretty clear when applied to the example in the article of code review. If the abstractions are designed and named well, why should the reviewer need to jump to the declaration of the function in the if statement? Assuming the checkHasPermissions function wasn’t added or modified in the PR, the reviewer should just consider whether it makes sense to do_something if and only if checkHasPermissions.

View on HN

You'd think so, but abstraction names are like comments: they are seldom updated when the implementation grows more hairs.

In older code, or code worked on by a larger team, I always chase into functions to understand the code, and don't rely on names. I'm also less inclined to indirect code unless the abstraction has genuine meaning, rather than being a compression technique for repetition, no matter how well named.

Then the problem here seems like the lack of drive towards intention-revealing naming. It's burdensome, but worth it when the intention is revealed and prevents the navigation to the implementation.

But like the author said, the reuse fallacy prevents this from having a good RoI until there is a significant usage of said abstraction.

> If the abstractions are designed and named well, why should the reviewer need to jump to the declaration of the function in the if statement?

How will the reviewer know if the abstraction is well-designed or named without reading it? Code quality attitudes change over time so sometimes the standard project-provided helpers end up not matching the intuitions of more recent contributors.

It’s a valid question, but at some point in a code review you have to trust an abstraction layer, lest you end up reading semiconductor physics papers for every PR.

Reply on news.ycombinator.com