Posts

Showing posts with the label Code Review

Typing != Complexity

Developers of all experience levels commonly share a false belief: more typing means more complexity. The most powerful idioms and tools often require a few extra keystrokes, but dramatically reduce complexity. The final keyword in Java is a bit of extra typing. The final keyword can be applied to variables, method parameters, instance members, static members, methods, and classes. By making these final, their values will not depend on their context. They will be known constants through their lifetime. Their constancy removes a large cognitive load. The developer no longer needs to worry if the value was not set or if it was set multiple times. Many developers forgo this benefits to avoid typing the extra keyword, to their own detriment. Later, they spend too much time chasing down defects caused by mutable state. Developers will often shun creating small types and interfaces because of the extra typing involved. Interfaces can abstract away many different implementations behind...

Untangling Nested Code

Image
There is code out there that is ugly. I mean really ugly. I know, can you believe it? But it's true. One such example of really ugly code is code that has lots of deeply nested calls. The tests are trying to test a dozen different methods in every case. The methods look so simple with just a few lines and a few calls. And then those methods called have a few lines and a few calls. And then again, a dozen or more different calls deep. We want to use private helper methods to make our code clearer and cleaner. Calling methods is also largely what programming is about. But sometimes we overdo it. Deep call hierarchies are difficult to follow and they can mask what the code is really doing. Each call means that we have to look all over a class to see what actually happens. This makes debugging hard. This makes testing even harder. Say we have the following code: class DataLoader { void loadNationalData(Nation nation, int year, DataReader reader) { for (State state ...

Making Wrong Code Be Wrong

I was recently perusing when I came across a link to a Joel Spolsky blog post . In it, he describes how a form of Hungarian notation can be used to make wrong code look wrong. In the process, he describes the interesting history of how Hungarian notation came to be, and how the common use of it today was not the way it was originally intended. He describes how Hungarian notation was created to convey application-level information about a variable. It could be used to distinguish between two different types of dimensions that might both be typed as int s. He termed this Apps Hungarian. While the compiler could not tell them apart, the human eye would be able to with a cursory glance. Later, the notation was instead conflated with the type that the compiler saw. An int iWidth carried redundant information that could actually get in the way if the type ever needed to change. He termed this Systems Hungarian. He gives several examples in the post. Notably, he describes how Apps Hunga...

Trapped in the Purgatory of a Big Refactoring

Sometimes, it's not just a method that needs refactoring, or even a class. Sometimes it's several large packages with a spider web of dependents that just need a complete "rehacktoring." This can happen when years of legacy code build up in a system. It is complicated by the fact that the thing you want to change or delete or rename or move has tendrils of dependents spread all across the system, sometimes into classloaders and reflection and dynamic scripts that you cannot begin to search-and-replace all of. It is going to be a big, long slog into the swamp. We don't want our massive change to be a surprise to our team, though. We want to commit and merge the bits as we're going along. They don't represent complete refactorings. We might have pulled out a class just to put some code in a separate place, knowing that down the road we're going to merge it together with some other class and move that other method somewhere else. We're not quite su...

YAGNI vs Uncertainty

You're sitting there staring at the (pretty good if you say so yourself) code you just wrote. It solves a pretty tricky problem in a pretty elegant way. But something is worrying you: this code might need to change. You're not sure when and you're not sure how, but you just get this feeling tingling in the back of your neck that this code is going to have to change, and probably sooner rather than later. Somehow sensing your hesitation from across the room, that guy comes over. Yo, bro, looks like you got a problem. Just remember, bro, YAGNI! You ain't gonna need all that! You've heard it all before from that guy . You've even followed his advice a couple of times. Once, yeah, he was right, but that other time... oh, that other time. So much extra work that you just know you could have avoided, somehow. Maybe I exaggerate a bit, but we've all seen or heard of or (shudder) worked with someone who strictly follows the One True YAGNI. They never add any co...

This One Subtle Bug You Might Not See Coming

Sometimes bugs are obvious. You used a ++ where you should have used a -- . Other times, bugs are much less obvious. A regular expression was missing an escape of a . . Then there are those bugs that are so subtle and insidious that they deserve their own blog post just to document and excoriate them. This post is about one of those bugs. Fortunately for me, this wasn't my bug. It was a bug hidden in a Programmers.SE question . The questioner was blaming himself for not seeing or understanding a change in an external library, and he was desperately looking for a way to write better code to account for such changes in the external library. What did that library do? There was a method that returned a long, where the long was milliseconds since epoch. In the update, the method was changed so that it still returned a long, but the long now represented nanoseconds since epoch. Putting aside the discussion about whether a long has enough precision, this change is a special type of a...

Learning to Refactor Other People's Code Therapeutically

StackExchange's CodeReview site provides a wealth of code samples representing the best, and sometimes the worst, of code that can be produced. It has snippets volunteered from all types of code bases in nearly every language. One could get lost for hours just reading how other people, both posters and answerers, choose to solve problems. It is fun to dive into some of the code on this site and work through refactoring it to see what comes out the other side. One such example I happened upon recently was this little gem . To summarize, it is a function that builds URLs from parts and puts those URLs into a dictionary object. It is written in Javascript for Node.JS. It uses a recursive function to permute the parts together to use as the dictionary keys, and for building those URLs. This is one of those pieces of code that fails the Squint Test badly. There is a lot of deep nesting, with for s inside of if s inside of for s. Given the indentation, the bottom half of the code ...