| At the risk of getting dinged, I have a hard time thinking this is a real article. Example 1: Changing `s` to `displayScore` doesn't fix the problem the code is using `p1`, `p2`, `p1N`, `p2N` AND defining a new `p`. WITH multiple ternaries and 3 mid-function returns. Example 2: Reasonable, but now you have two functions with similar signatures. Reducing the number of indentation is always good. Example 3: Creates ANOTHER layer of indentation. Without add'l context the code looks like it could be reduced to a series of inverted conditions with happy/golden-path style sequential if-s instead of 5 indentation deep. Example 4: Pet peeve and chided by a lot of linters. Empty blocks are generally frowned upon. (You say: "If not equals DO" not "If equals don't do anything ELSE") Example 5: Good in the context of testing, annoying in that it is a function with a single line. You'd probably be better off stubbing the User class all together. I mean you're already providing a User-type object. Example 6: Great, 10/10 love it. Would remove the {} and just toss the Throws on the if-statement line itself but that's personal taste. Example 7: Not awful, but I like to front-load my variable declarations so you can get error-handling out of the way at the start of the function then use those variables to do the computation later. More up front about saying: "Here are the items I will use later in this function" Example 8: Probably not a simple refactor. If you aren't going to update all the locations in which an "Amount"/"Currency", you're creating an artisanal class for a single use. Why am I code reviewing this article? Shoo. |
Sorry if you had a hard time to read this post.
Your comment for the in Example #1 is right, we've shown only the first step. The second one would be to rename all others code operations: => Renaming `p1`, `p2`, `p1N`, `p2N => Remove the magic numbers 4, 6 and 1.
One important thing in this article is the fact that each tip should be used by a developer locally as an intermediate step during a refactoring of a legacy code. So for each tip, the second piece of code should not be the final version to be committed. It clearly needs additional operations and transformations.
We'll do our best to make our next post easier to digest :)