top | item 17986844

(no title)

lsadam0 | 7 years ago

It sounds like you really just disagree with the example given, not necessarily the rule itself. You're arguing that `isAdmin()` might require a different implementation in a difference scenario, but you have not made a case that the string "admin" should not be constant or enum.

I honestly cannot think of a reasonable argument for why constant values should not be in static constants or enums. For one, why would you want to re-type the value over and over? And two, it only takes one typo in one manually typed value to introduce a frustrating bug! It's such an easy bug to avoid and costs almost zero to do correctly! Honestly, devs manually typing values that are effectively consts is one of my biggest pet peeves, you're just creating easily avoidable problems for yourself and your team :).

discuss

order

kevinconaway|7 years ago

> I honestly cannot think of a reasonable argument for why constant values should not be in static constants or enums.

My personal opinion is that values should be in constant values if:

- Its repeated more than once or referenced outside of the file

- The constant value is a "magic number" where the meaning of the value isn't obvious to outsider

If the value is used only once and the meaning of the value is clear, I think extracting a constant out is needless indirection.

For example:

  if (flags & 45676 == 0) {}
is a magic number that definitely should be a constant, even if its only used once:

  if (flags & ROLE_ADMIN == 0) {}
However something like:

  pool.setDefaultTimeoutMillis(1000);
is perfectly obvious in context IMO

lsadam0|7 years ago

I will say your take is reasonable, but on projects I manage I still push for usage of const/enum in these cases.

codingdave|7 years ago

I'm actually arguing that we should not be focused on nitpicking the details, and instead appreciating the effort taken to think things through, and supporting the idea of using the article as a springboard for our own improvements.