To read more articles like this, visit my blog
We are always so excited about the new shiny things that come out every week in the programming world. New ways to structure components, new techniques to reduce those two lines of code, and so on.
But in the real world, things are not so shiny. We often have to deal with codebases that have been evolving for many years, and many developers have left their mark on the components' different parts.
Our story is about a component that has 2700 lines of code. Let’s try to explain how things went south and how we can do better.
Background
I worked in a company with a fleet management tool dashboard showing vehicles roaming around the city in real-time.
The dashboard component is the hero of our story today. It has many functionalities. Nothing fancy, though.
A map that shows the vehicle markers
A way to search through the vehicles
A popup that shows vehicle details when clicked
A list of vehicles at the bottom
Some filtering options.
Sure, there are several features that this single component is responsible for displaying. But is it that much to contain 2700 lines of code?
The Component
Obviously, I will be stupid to paste 2700 lines of code into this article. (Also, it will be illegal :P ) But let me show you the general structure of the component.
// import statements -> 85 lines
import React from 'react';
import styled from 'styled-components'
...
...
--------------
//constant declaration -> 15 lines
const MAX_LOOP= 40;
const FAILURE_MESSAGE = "Something Brutal Happened!"
...
...
--------------
//styled component declaration -> 580 lines!!!!!
const SomeStyle = styled.div`
display:flex;
flex-direction: column;
`
...
...
---------------
//Helper components -> 200 lines
const MapPortal = () => {
return <div>
// .. some placeholder component
</div>
}
...
...
---------------
// Props and State Type Declaration -> 70 lines
type Props = {
vin: string;
.....
}
type State = {
mapVisibility: boolean;
....
}
---------------
// The actual component with all it's helper methods -> 1700 lines!!!!!!!!!!!!!!!!!!
export class TheActualComponent extends React.Component<Props , State>{
constructor(props: Props){
// ...
}
componentDidMount() {
// ... And other lifecycle methods
}
randomHelperMethod(){
// These functions extracts some logic into a separate function
}
onClickHandlers() {
// Lots of them!
}
render(){
return <div>
// {Something worth of it?}
</div>
}
}
// redux+type declarations -> 200 lines
export const mapStateToProps = {
// .. lots of them!
}
export const mapDisptachToProps = {
// .. lots of them
}
export Component
Okay Now Tell Me What’s Wrong Here
To be 100% honest? Almost everything. Let me explain those.
1. Constant Declaration
This is such an obvious one. Unfortunately, I have seen many examples of it throughout many companies and many components.
You should Keep the Constants in a Separate File
It doesn’t matter if they are being re-used or not. It’s still better to store them in a separate File. Because down the line, someone else would create a separate constant with the same value, eventually creating confusion.
I do it this way.
// for related constants
export DeclaredConstants {
SOME_CONSTANT_1 = "SomethingSpecial1";
SOME_CONSTANT_2 = "SomethingSpecial2"
}
// or for single constants
export const SOME_CONSTANT_3 = "SomethingSpecial3"
And then import the inside my component and use them.
import { DeclaredConstants } from "ConstantDeclaration.ts"
console.log(DeclaredConstants.SOME_CONSTANT_1)
This particular component isn’t a big deal, but best practices are best practices.
2. Styles and Helper Methods
I think it’s a very common mistake and (sometimes allowable) for smaller components to put the styles and helper methods in the same file.
If your component is only 30–50 lines of code, then it can make sense to keep the styles and helper methods in the same file.
But having 580 lines of style declaration doesn’t make sense in any scenario. Because not very often you would need to touch these styles.
What I do is follow the following folder structure to keep things organized.
-component-name
|-- __tests__
|-- styles.ts
|-- helpers.ts
|-- index.ts
The responsibility of the files is clear from the name of the files themselves. It’s so simple to simply split our massive component to 1/3rd of its current size just by putting things where they should be!
An Example
export const SomeStyle = styled.div`
display: flex;
`
If you are using Raw CSS or SCSS, then you probably don’t make this mistake, but the projects using styled-components or material-ui mostly follow this bad practice.
Once someone started it. It became the standard practice.
So don’t fall into this trap. Create a separate file for styles ahead of time that can save your component in the future.
3. Dumb Components
There are two types of components.
Dumb components -> only act as a container or view
Intelligent components -> Shows something based on some logic
Now there is no reason to put two components in the same file. It directly violates the Single Responsibility Principle.
Every class or component should do one thing and one thing only
Sometimes we get lazy (Myself included) and put simple container components into the actual component. But what do you think when the next developer comes?
Does he take out the smaller component into its own file?
Umm… Probably not. So after 4–5 years, you now have 200 lines of helper dumb components that can easily be extracted from separate files.
4. Class Component
Not sure if you noticed, but this Massive component is using ClassComponent. I’m sure you know the reason. It was written at a time when functional components were not that common.
But nowadays, using functional components make more sense because
They are easy to maintain
Less code
More performant? (Arguably)
But even I wouldn’t try to convert it into a functional component at this stage. We need to refactor many things before converting this into a functional component.
Let’s see how much we could gain.
Let me show you an estimation of how much we can improve without even understanding what this component is doing.
We can just export the constants to constants.ts , Styles into the styles.ts and helper methods into the helpers.ts file.
Styles = 600
Constants(Types+Others) = 100
Helper Methods. = 600 (roughly)
Dumb Components = 200
---------------------------------
Total Gain = 1400 lines!
This is a work of two hours maximum. All we need to do is.
Putting things into appropriate files and import them
We can reduce a component from 2700 to 1300 lines!
Some might say that it’s still a lot. But hey!!! One step at a time, right?
Can we do better?
Yes, of course. When we look into the internal logic, we can do even better.
Break the actual components and re-usable parts into even smaller components
Using Functional component
Taking advantage of hooks
Using functional redux
And so on….. But that is a story for another day.
Show me the Good Parts.
Obviously, This component has lots of problems but some good things.
Typescript
Although the Type declarations add up to around 200 lines in total, they are worth it. Especially without Typescript, it would be impossible to maintain this component.
Extracting logic
Some of the dumb logic is extracted out of the view logic itself. For example, showing a message based on the vehicle status.
showVehicleStatusMessage (status: VehicleStatus){
switch(status): {
case SOME_STATUS:
return "Some Message"
case SOME_OTHER_STATUS:
return "Some Other Message"
}
}
It’s better to have them in a separate function like this instead of writing the logic into the view. Which can look something like this…
return (
{vehicleStatus === SOME_STATUS && <div> {"Some Message"} </div>
)
So it’s not all bad; some developers tried to do things correctly. At the end of the day, software development is a team effort.
What did I learn?
The most important takeaway for me is to understand the importance of following the best practices.
Best practices are there for a reason
The impact of following best practices may not be evident on the first day, but if you don’t follow those, you will feel the pain someday!
Conclusion
I hope you learned something new today. Have a great day!
Have something to say? Get in touch with me via LinkedIn or Personal Website
Top comments (1)
Absolutely amazing and relevant article! I deal with this all the time when I'm working on legacy code.
It's fascinating really how much time and effort you would save if you just apply the simplest of principles and best practices such as separation of concerns and modularity.
I don't think it's just laziness though. It's partly laziness but also the fact that developers have so many requirements that once they get something working they think "good, I'll just optimize it later" and move on to the next thing.
The "broken window" analogy in the book Clean Code best summarizes that thought process in my opinion.
Again, thanks for the post!