undefined | Better HN

0 pointsQuarrelsome3mo ago0 comments

> Naming comments can be very useful in code that gets read by a lot of people. It can make the process of understanding the code much quicker.

yes but it can be severely diminishing returns. Like lets step back a second and ask ourselves if:

var itemCount = items.Count;

var numberOfItems = items.Count;

is ever worth spending the time discussing, versus how much of a soft improvement it makes to the code base. I've literally been in a meeting room with three other senior engineers killing 30 minutes discussing this and I just think that's a complete waste of time. They're not wrong, the latter is clearer, but if you have a PR that improves the repo and you're holding it back because of something like this, then I don't think you have your priorities straight.

0 comments

hinkley3mo ago

Generally you’d like the variable to imply a call to action. Even if the call to action is for a feature still in the backlog.

Over time I’ve developed some tricks that invite people to add features to the code in the “right” place, and this is one of them. Once in a while someone gets credit for work I already thought to do but didn’t have time. But for every one of those there’s a half dozen or a dozen cases of increasing the bus number on a block of code I wrote be nerd sniping people into making additions while I’m busy with something else.

spooky_action3mo ago

Sorry for the dumb question, is the second version actually better than the first? Because I prefer the first. But perhaps you chose this as a particularly annoying/unuseful comment

QuarrelsomeOP3mo ago

I personally don't give a shit either way but I've worked in dev shops with a clear preference for the second one. I can see their point because the code as natural language parses better but I don't think its strong enough to care about.

Sort of place that is fussy about test naming so where I would do smth like:

TestSearchCriteriaWhere

they'd want

Test_That_Where_Clauses_In_Search_Criteria_Work

I think its a waste of typing but idk, I'm willing to let it slide because I think its a pointless hill to die on.

1 more reply

solid_fuel3mo ago

If I was going to nitpick it I would point out that `itemsCount` could easily be confused with `items.Count`, or vice versa, depending on syntax highlighting. That kind of bug can have a negative impact if one or the other is mutated while the function is running.

So clearly distinguishing the local `numberOfItems` from `items.Count` _could_ be helpful. But I wouldn't ping it in a review.

1 more reply

ambicapter3mo ago

They’re both equally bad to me, I don’t see the improvement over just using item.count. I may be nitpicking a toy example though.

1 more reply

j / k navigate · click thread line to collapse

0 comments

hinkley3mo ago

Generally you’d like the variable to imply a call to action. Even if the call to action is for a feature still in the backlog.

spooky_action3mo ago

Sorry for the dumb question, is the second version actually better than the first? Because I prefer the first. But perhaps you chose this as a particularly annoying/unuseful comment

QuarrelsomeOP3mo ago

Sort of place that is fussy about test naming so where I would do smth like:

TestSearchCriteriaWhere

they'd want

Test_That_Where_Clauses_In_Search_Criteria_Work

I think its a waste of typing but idk, I'm willing to let it slide because I think its a pointless hill to die on.

1 more reply

solid_fuel3mo ago

So clearly distinguishing the local `numberOfItems` from `items.Count` _could_ be helpful. But I wouldn't ping it in a review.

1 more reply

ambicapter3mo ago

They’re both equally bad to me, I don’t see the improvement over just using item.count. I may be nitpicking a toy example though.

1 more reply

j / k navigate · click thread line to collapse