5 easy wins to refactor even the ugliest code

Writing clean code can be a challenge when you start on a new project. Trying to clean up code in an already existing application without breaking anything is similar to this: I have been a technical lead for a few years and during that time I have seen my fair share of spaghetti code that I had to maintain, or worst extend. Through many painstakingly frustrating hours, and a dozen rubber ducks or so I have developed a few tips and tricks to help refactor even the ugliest code bases. I put together a list of 5 quick wins that you can either apply to any code base or keep in mind if you're starting a new project. Each win will start with a bad code sample followed by a good one with an explanation of what changed. 1) No Invariant Methods A method can be considered pretty much useless if it always returns the exact same thing no matter what is passed in. As you can see in the example bad example, getName will always return the exact same "NO NAME FOUND" message. We fix this in the good example method by simply returning the result that we intended to from the get go. 🚫 public getName(): string { result = ; ( .name) { result = .name; } ; } let "NO NAME FOUND" if this this return "NO NAME FOUND" ✅ public getName(): string { result = ; ( .name) { result = .name; } result; } let "NO NAME FOUND" if this this return 2) No magic numbers or strings Magic numbers and strings are hardcoded values that you can find sprinkled throughout your code. If not done carefully you will end up repeating the exact same values across your entire code base, which will make changing your code, debugging and testing it a nightmare. As you can see below we have a hardcoded string in our method. In the file right below it, we fixed this issue by refactoring it to a private read only class variable. 🚫 public getName(): string { result = ; ( .name) { result = .name; } result; } let "NO NAME FOUND" if this this return ✅ { private readonly NAME_DEFAULT = ; private name; public getName(): string { result = .NAME_DEFAULT; ( .name) { result = .name; } result; } } export class User "NO NAME FOUND" let this if this this return A good question to ask regarding magic numbers and strings, is "Where should I put them?". Here is how I decide where to put them: 1. Is the value only in one file? 👉 a constant in the top of the file 2. Is the value used across multiple files? 👉 a specific enum class that groups the constants logically 3. Is the value used when the application boots (e.g. an api url, a timeout threshold…) 👉 a properties file pssst I tweet about code stuff all the time. If you have questions about how to level up your dev skills give me a follow @mlevkov 3) Keep the default clause last A switch statement can contain a default clause to handle unexpected value. Considering you are most likely working collaboratively on a code base, it's important to follow conventions to ease communication. Considering the example below, just moving the default clause all the way to the bottom will already save a lot of frustration for your peers. 🚫 { Track } ; { dimension: string; url: string; } { getBackgroundArt(track: Track): BackgroundImage { (track.getGenre()) { : ; : hipHopImage: BackgroundImage = { : , : }; hipHopImage; : jazzImage : BackgroundImage = { : , : }; jazzImage; : rapImage : BackgroundImage = { : , : }; rapImage; : countryImage: BackgroundImage = { : , : }; countryImage; } } } import from "../models/Track" export class BackgroundImage export class BackgroundImageService switch default return null case "hiphop" const dimension 'small' 'url' 'https://unsplash.com/photos/Qcl98B8Bk3I' return case "jazz" const dimension 'small' 'url' 'https://unsplash.com/photos/dBWvUqBoOU8' return case "rap" const dimension 'small' 'url' 'https://unsplash.com/photos/auq_QbyIA34' return case "country" const dimension 'small' 'url' 'https://unsplash.com/photos/RnFgs90NEHY' return ✅ getBackgroundArt(track: Track): BackgroundImage { (track.getGenre()) { : hipHopImage: BackgroundImage = { : , : }; hipHopImage; : jazzImage : BackgroundImage = { : , : }; jazzImage; : rapImage : BackgroundImage = { : , : }; rapImage; : countryImage: BackgroundImage = { : , : }; countryImage; : ; } } switch case "hiphop" const dimension 'small' 'url' 'https://unsplash.com/photos/Qcl98B8Bk3I' return case "jazz" const dimension 'small' 'url' 'https://unsplash.com/photos/dBWvUqBoOU8' return case "rap" const dimension 'small' 'url' 'https://unsplash.com/photos/auq_QbyIA34' return case "country" const dimension 'small' 'url' 'https://unsplash.com/photos/RnFgs90NEHY' return default return null 4) Clean up your redundant variables Keep an eye out when you don't need to instantiate a variable at all. Redundant variables take up memory, clutter up your code, make it harder to debug and add new features. Try to keep your code as tidy as possible. In the examples below, it's painfully obvious we don't need to create an instance of a BackgroundImage each time, and can just assign the value for each case. 🚫 getBackgroundArt(track: Track): BackgroundImage { (track.getGenre()) { : hipHopImage: BackgroundImage = { : , : }; hipHopImage; : jazzImage : BackgroundImage = { : , : }; jazzImage; : rapImage : BackgroundImage = { : , : }; rapImage; : countryImage: BackgroundImage = { : , : }; countryImage; : defaultImage: BackgroundImage = { : , : }; defaultImage; } } switch case "hiphop" const dimension 'small' 'url' 'https://unsplash.com/photos/Qcl98B8Bk3I' return case "jazz" const dimension 'small' 'url' 'https://unsplash.com/photos/dBWvUqBoOU8' return case "rap" const dimension 'small' 'url' 'https://unsplash.com/photos/auq_QbyIA34' return case "country" const dimension 'small' 'url' 'https://unsplash.com/photos/RnFgs90NEHY' return default const dimension 'small' 'url' 'https://unsplash.com/photos/PDX_a_82obo' return ✅ getBackgroundArt(track: Track): BackgroundImage { backgroundImage: BackgroundImage; (track.getGenre()) { : backgroundImage = { : , : }; ; : backgroundImage = { : , : }; ; : backgroundImage = { : , : }; ; : backgroundImage = { : , : }; ; : backgroundImage = { : , : }; } backgroundImage; } let switch case "hiphop" dimension 'small' 'url' 'https://unsplash.com/photos/Qcl98B8Bk3I' break case "jazz" dimension 'small' 'url' 'https://unsplash.com/photos/dBWvUqBoOU8' break case "rap" dimension 'small' 'url' 'https://unsplash.com/photos/auq_QbyIA34' break case "country" dimension 'small' 'url' 'https://unsplash.com/photos/RnFgs90NEHY' break default dimension 'small' url 'https://unsplash.com/photos/PDX_a_82obo' return 5) Forget the else, return if you can An important concept in keeping code clean is cyclomatic complexity. Cyclomatic complexity is basically how many loops, conditionals or function calls do you have nested in our code. Really high cyclomatic complexity makes code nearly impossible to wrap your head around. You will definitely need a dozen rubber ducks if it gets out of control. Let's simplify the example from above by going from a switch statement to a tidy series of ifs. 🚫🚫 getBackgroundArt(track: Track): BackgroundImage { backgroundImage: BackgroundImage; (track.getGenre()) { : backgroundImage = { : , : }; ; : backgroundImage = { : , : }; ; : backgroundImage = { : , : }; ; : backgroundImage = { : , : }; ; : backgroundImage = { : , : }; } backgroundImage; } let switch case "hiphop" dimension 'small' 'url' 'https://unsplash.com/photos/Qcl98B8Bk3I' break case "jazz" dimension 'small' 'url' 'https://unsplash.com/photos/dBWvUqBoOU8' break case "rap" dimension 'small' 'url' 'https://unsplash.com/photos/auq_QbyIA34' break case "country" dimension 'small' 'url' 'https://unsplash.com/photos/RnFgs90NEHY' break default dimension 'small' url 'https://unsplash.com/photos/PDX_a_82obo' return 🚫 getBackgroundArt(track: Track): BackgroundImage { backgroundImage: BackgroundImage; (!track.getGenre()) { backgroundImage = { : BackgroundImageDimensions.small, : .DEFAULT_BACKGROUND_IMAGE_URL}; } (track.getGenre() == ) { backgroundImage = { : BackgroundImageDimensions.small, : .HIPHOP_BACKGROUND_IMAGE_URL}; } (track.getGenre() == ) { backgroundImage = { : BackgroundImageDimensions.small, : .JAZZ_BACKGROUND_IMAGE_URL}; } (track.getGenre() == ) { backgroundImage = { : BackgroundImageDimensions.small, : .RAP_BACKGROUND_IMAGE_URL}; } (track.getGenre() == ) { backgroundImage = { : BackgroundImageDimensions.small, : .COUNTRY_BACKGROUND_IMAGE_URL}; } backgroundImage; } let if dimension url this else if "hiphop" dimension url this else if "jazz" dimension url this else if "rap" dimension url this else if "country" dimension url this return ✅ getBackgroundArt(track: Track): BackgroundImage { backgroundImage: BackgroundImage = { : BackgroundImageDimensions.small, : .DEFAULT_BACKGROUND_IMAGE_URL}; (track.getGenre() == ) { { : BackgroundImageDimensions.small, : .HIPHOP_BACKGROUND_IMAGE_URL}; } (track.getGenre() == ) { { : BackgroundImageDimensions.small, : .JAZZ_BACKGROUND_IMAGE_URL}; } (track.getGenre() == ) { { : BackgroundImageDimensions.small, : .RAP_BACKGROUND_IMAGE_URL}; } (track.getGenre() == ) { { : BackgroundImageDimensions.small, : .COUNTRY_BACKGROUND_IMAGE_URL}; } backgroundImage; } let dimension url this if "hiphop" return dimension url this if "jazz" return dimension url this if "rap" return dimension url this if "country" return dimension url this return This last example keeps the flow of logic dead simple. There is no alternative flows that you have to map out, everything is very straight forward and becomes much easier to digest. There you have it, 5 easy tips that you can apply to almost any codebase. If you want to level up your coding skills, I'm putting together a playbook that includes: 1. 30+ common code smells & how to fix them 2. 15+ design pattern practices & how to apply them 3. 20+ common JS bugs & how to prevent them Get early access to the 🚀 Javascript playbook