First, I was exaggerating for humor, partly to make fun of the example and partly or make fun of programmers who would criticize even three lines of code out of context, because they like to feel smart. 😉
Even so, if I played “What’s not to like about this code?” I would point out a few things to look at more closely.
Writable shared memory always draws my attention. It’s not bad, but I tend to prefer to minimize it. A writable field in an object is often perfectly fine and sometimes it’s a signal of a poorly-drawn boundary between methods.
Writing to a field, then immediately returning falls into a similar category. Often this is perfectly fine, but sometimes it’s a symptom of a misdrawn boundary between methods. I would prefer a function or method that calculates and returns the value, then separate code that assigns it to a field. That makes the calculation easier to inspect, such as with a test.
Naming a boolean value for the “no” state always feels strange to me. I always prefer to name it for the “yes” state, so I would challenge the name invalid for that field.
Having a boolean value to represent that a thing is valid tends to be a step in the direction of a little parser. Often, the design gets stuck at a spot where there is a boolean combined with another variable in such a way that the values of those variables is commented in some way. That design can almost always be improved by introducing a little parser that returns Maybe or Either or throws an exception. (https://blog.thecodewhisperer.com/permalink/a-guard-clause-is-maybe-a-tiny-parser)
Naming an argument with the prefix arg is usually either following an obsolete habit purely out of habit or a survival tactic for identifying arguments to loooooong functions. Sometimes we do this because we have to, but I would almost always prefer to split up the function to make this naming convention unhelpful and unnecessary.
And it’s very important to clarify that I raise these as areas of interest for further attention. They are at most risks. I can think of good reasons to do all these things, but I would tend to treat them as very temporary choices that I’d probably prefer to change sooner than later.
Was any of that helpful at all? Any further questions about it?
First, I was exaggerating for humor, partly to make fun of the example and partly or make fun of programmers who would criticize even three lines of code out of context, because they like to feel smart. 😉
Even so, if I played “What’s not to like about this code?” I would point out a few things to look at more closely.
Writable shared memory always draws my attention. It’s not bad, but I tend to prefer to minimize it. A writable field in an object is often perfectly fine and sometimes it’s a signal of a poorly-drawn boundary between methods.
Writing to a field, then immediately
returning falls into a similar category. Often this is perfectly fine, but sometimes it’s a symptom of a misdrawn boundary between methods. I would prefer a function or method that calculates and returns the value, then separate code that assigns it to a field. That makes the calculation easier to inspect, such as with a test.Naming a boolean value for the “no” state always feels strange to me. I always prefer to name it for the “yes” state, so I would challenge the name
invalidfor that field.Having a boolean value to represent that a thing is valid tends to be a step in the direction of a little parser. Often, the design gets stuck at a spot where there is a boolean combined with another variable in such a way that the values of those variables is commented in some way. That design can almost always be improved by introducing a little parser that returns Maybe or Either or throws an exception. (https://blog.thecodewhisperer.com/permalink/a-guard-clause-is-maybe-a-tiny-parser)
Naming an argument with the prefix
argis usually either following an obsolete habit purely out of habit or a survival tactic for identifying arguments to loooooong functions. Sometimes we do this because we have to, but I would almost always prefer to split up the function to make this naming convention unhelpful and unnecessary.And it’s very important to clarify that I raise these as areas of interest for further attention. They are at most risks. I can think of good reasons to do all these things, but I would tend to treat them as very temporary choices that I’d probably prefer to change sooner than later.
Was any of that helpful at all? Any further questions about it?
Yes this was a super interesting read as were most of the other replies. Thanks for sharing some info!
I glad it helped! Peace.