Contents
Outline
This post shares the content from a video presented at SLASH, a conference held by the Korean fintech company Toss.
Toss is a Korean fintech startup that has since gone public and is now one of the top companies Korean developers want to work at.
When we think of Clean Code, we tend to think of readable names and reducing duplication, but in practice, more skills are needed to write readable code.
Here we share concepts and action items for writing readable code on the frontend.
The example code is in React. Since it’s not working code, it’s best to focus on understanding the concepts.
Definition of Clean Code in Practice
Besides developer self-satisfaction, what is the meaning of Clean Code in practice?
You’d better not touch that code.
Let me fix it for now.
You’ve probably heard these phrases at work. Most companies have this kind of landmine code.
These phrases contain the following implications:
It's hard to understand the flow and meaning of the codeThe domain context is not well expressedYou can't understand it without asking another team member
This landmine code becomes a bottleneck during development and takes a long time to understand during maintenance.
In the worst case, it may become impossible to add new features.
Also, such code often has poor performance, which leads to bad user experiences.
The meaning of Clean Code in practice = Reducing maintenance time
Clean Code in practice means reducing maintenance time.
If you can quickly understand code written by other team members or by your past self, development time during maintenance becomes shorter.
Reducing maintenance time = Reducing time for code comprehension, debugging, and review
Readable, clean code reduces both code review time and debugging time when bugs occur.
Time = Resources = Money
Time is resources, and resources are money. If there’s code that takes 1 day to fix versus code that takes 3 days, simply put, the 3-day code requires 3 times more developers or 3 times more effort. In other words, we as developers have to work 3 times harder.
The Trap of Code Addition
As professional developers, we can write very clean code when designing and building from scratch.
However, when adding features to existing code, the story changes. If we lose focus even slightly, the code deteriorates.
90% of our work is adding features to existing code. We add features to code written by other team members, or code we wrote last week.
Here’s the actual work the presenter received:
There was a page where users could enter insurance-related questions, and the feature request was: if the user has an insurance agent, show a popup with the agent’s photo.
Design and Implementation
The existing code looked like this:
function QuestionPage() {
async function handleQuestionSubmit() {
// Check terms of service agreement
const agree = await getTargetElement();
if (!agree) {
// If agreement is needed, show popup
await openAgreePopup();
}
// For users who agreed to terms, register the question and show success message
await sendQuestion(questionValue);
alert('Your question has been registered.');
}
return (
<Main>
<form>
<textarea placeholder="Enter your question" />
<button onClick={handleQuestionSubmit}>Submit Question</button>
</form>
</Main>
);
}How should we add the new feature to this code? It seems straightforward to add it like this:
[Design]
function QuestionPage() {
async function handleQuestionSubmit() {
/*
* [Check if user has an insurance agent,
* and if so, add logic to show popup]
*/
// Check terms of service agreement
const agree = await getTargetElement();
if (!agree) {
// If agreement is needed, show popup
await openAgreePopup();
}
// For users who agreed to terms, register the question and show success message
await sendQuestion(questionValue);
alert('Your question has been registered.');
}
return (
<Main>
<form>
<textarea placeholder="Enter your question" />
<button onClick={handleQuestionSubmit}>Submit Question</button>
/* * [Add popup component. Hidden by default, shown when needed] */
</form>
</Main>
);
}[Development]
function QuestionPage() {
const [popupOpened, setPopupOpend] = useState(false); // Popup state
async function handleQuestionSubmit() {
// If user has an agent, show popup
const myExpert = await getMyExpert();
if (myExpert !== null) {
setPopupOpened(true);
} else {
// Check terms of service agreement
const agree = await getTargetElement();
if (!agree) {
// If agreement is needed, show popup
await openAgreePopup();
}
// For users who agreed to terms, register the question and show success message
await sendQuestion(questionValue);
alert('Your question has been registered.');
}
}
async function handleMyExpertQuestionSubmit() {
await sendQuestionToMyExpert(questionValue, expert.id);
alert(`Asked ${myExpert.name} a question.`);
}
return (
<Main>
<form>
<textarea placeholder="Enter your question" />
<button onClick={handleQuestionSubmit}>Submit Question</button>
{popupOpened && (
<MyExpertPopup onSubmit={handleMyExpertQuestionSubmit} />
)}
</form>
</Main>
);
}We developed it as designed.
We added a State to track whether the popup is displayed, added an if statement to the existing click function to check if the user has an insurance agent.
We created a function to be called when the confirm button is pressed in the insurance agent popup, and added the popup component.
Problems
This looks like correct, natural code addition, but it’s actually become bad code.
-
Code with a single purpose is scattered everywhere. The code below is related to the insurance agent and popup, but since this code is spread apart, you need to scroll around and check multiple places when adding features later.
- popupOpened State
- Agent check and popup display logic
- Function to send data to the agent
- Agent popup component
-
A single function is doing multiple roles. The existing function has 3 roles. You can’t understand the function’s role without reading all of its contents. This means adding or removing code takes even more time.
-
The detail levels of function implementations differ. The
handleQuestionSubmitandhandleMyExpertQuestionSubmitfunctions are event handling functions. Their names are similar —handleQuestionSubmitandhandleMyExpertQuestionSubmit— buthandleQuestionSubmitdoes many things beyond submitting questions, making it hard to understand without looking at the code overall. Since you can no longer predict the code’s behavior from the name alone, you either have to check everything or risk misunderstanding.
The original code was good, but just one feature addition made it hard to understand.
The trap here is that when viewed in a PR, it’s hard to recognize this as hard-to-read code.
The reason is that when you only look at the changes, it doesn’t appear to be bad code. But looking at it as a whole, it’s a mess.
The After code is what the presenter actually submitted in their first PR.
Solution (Refactoring)
The problems can be solved through the following refactoring.
- Unified the implementation detail levels of functions.
Changed the existing function name from
handleQuestionSubmittohandleNewExpertQuestionSubmit, and added thehandleMyExpertQuestionSubmitfunction to unify the function hierarchy.handleNewExpertQuestionSubmitcontains only the logic for sending to a new agent.handleMyExpertQuestionSubmitcontains only the logic for sending to an already connected agent.
function QuestionPage() {
const myExpert = useFetchMyExpert();
async function handleNewExpertQuestionSubmit() {
await sendQuestion(questionValue);
alert("Your question has been registered.");
}
async function handleMyExpertQuestionSubmit() {
await sendQuestionToMyExpert(questionValue, expert.id);
alert(`Asked ${myExpert.name} a question.`);
}-
Consolidated popup-related code into one place.
- Previously, the button to open the popup and the popup code were separated. We combined them into a
PopupTriggerButtoncomponent.
- Previously, the button to open the popup and the popup code were separated. We combined them into a
-
Split functions so each has a single responsibility.
- Created the terms agreement function as
openPopupToNotAgreeUsers, to be called when needed.
- Created the terms agreement function as
function QuestionPage() {
const myExpert = useFetchMyExpert();
async function handleNewExpertQuestionSubmit() {
await sendQuestion(questionValue);
alert('Your question has been registered.');
}
async function handleMyExpertQuestionSubmit() {
await sendQuestionToMyExpert(questionValue, expert.id);
alert(`Asked ${myExpert.name} a question.`);
}
async function openPopupToNotAgreeUsers() {
// Check terms of service agreement
const agree = await getTargetElement();
if (!agree) {
// If agreement is needed, show popup
await openAgreePopup();
}
}
return (
<Main>
<form>
<textarea placeholder="Enter your question" />
<button onClick={handleQuestionSubmit}>Submit Question</button>
{myExpert.connected ? (
<PopupTriggerButton
popup={<MyExpertPopup onSubmit={handleMyExpertQuestionSubmit} />}
>
Ask Question
</PopupTriggerButton>
) : (
<Button
onClick={async () => {
await openPopupToNotAgreeUsers();
await handleNewExpertQuestionSubmit();
}}
>
Ask Question
</Button>
)}
</form>
</Main>
);
}What Is Clean Code in Practice?
The code has become longer than the first version.
Clean Code != Short code
Clean Code in practice is not short code, but code where you can quickly find the logic you need.
Clean Code == Code where you can quickly find the logic you need
Code Where Logic Is Easy to Find
To be able to quickly find the logic you need: when code with a single purpose is scattered around, you need to increase cohesion and consolidate it; when a function does multiple things, you need to split it following the Single Responsibility Principle.
And when function implementation detail levels differ, you need to adjust the abstraction level to expose important concepts only as much as needed.
Code where logic is easy to find:
- Code with a single purpose is scattered → Cohesion
- A function does multiple things → Single Responsibility
- Function implementation detail levels are inconsistent → Abstraction
Let’s look at actual code and explain each one.
Cohesion
Let’s consolidate code with the same purpose.
Here’s actual code. The code that operates the popup is split across 3 places.
function QuestionPage() {
const [popupOpened, setPopupOpened] = useState(false);
async function handleClick() {
setPopupOpened(true);
}
function handlePopupSubmit() {
await sendQuestionToMyExpert(questionValue, expert.id);
alert(`Asked ${myExpert.name} a question.`);
}
return (
<>
<button onClick={handleClick}>Ask Question</button>
<Popup title="Insurance Question" open={popupOpened}>
<div>The agent will explain</div>
<button onClick={handlePopupSubmit}>Confirm</button>
</Popup>
</>
)
}The code is hard to understand at a glance and has a high risk of bugs.
Refactor V1
Used a Custom Hook to consolidate everything in one place.
function QuestionPage() {
const [openPopup] = useMyExpertPopup(myExpert.id);
async function handleClick() {
openPopup();
}
return <button onClick={handleClick}>Ask Question</button>;
}Now you can open the popup by calling the openPopup function.
However, looking carefully, this code has become hard to read.
What popup is displayed and what action happens when the popup button is pressed are the most important points on this page, but they’re all hidden in hooks and can’t be seen.
This is a typical anti-pattern of Custom Hooks. When you see messy code, you just wrap everything in hooks.
So what should we consolidate?
Implementation details that you don’t need to know right away. Hiding these lets you quickly understand the code’s purpose from just the short code.
Conversely, things that become harder to see when consolidated are essential information for understanding the code. Separating these out means you’ll be going back and forth between modules to track the code flow.
Clean Code != Short code
Making short code by consolidating doesn’t make it Clean Code.
Clean Code is code where you can quickly find the logic you’re looking for.
How can we consolidate in a readable way?
Code Cohesion Tip: Separate core data from implementation details
First, let’s separate core data that should remain visible from implementation details that can be hidden.
In this code, the core data is the action to execute when the popup button is clicked, and the popup’s title and content.
function QuestionPage() {
/*
* Implementation detail: popup button click action
*/
const [popupOpened, setPopupOpened] = useState(false);
async function handleClick() {
setPopupOpened(true);
}
function handlePopupSubmit() {
/*
* Core data: popup button click action
*/
await sendQuestionToMyExpert(questionValue, expert.id);
alert(`Asked ${myExpert.name} a question.`);
}
return (
<>
<button onClick={handleClick}>Ask Question</button>
/*
* Core data: title, content
* Implementation detail: component Markup and calling function on button click
*/
<Popup title="Insurance Question" open={popupOpened}>
<div>The agent will explain</div>
<button onClick={handlePopupSubmit}>Confirm</button>
</Popup>
</>
)
}The implementation details are the State for opening/closing the popup, the component’s detailed Markup, and the binding to call a specific function when the popup button is clicked.
Refactor V2
If we keep the core data and hide the implementation details, the code becomes easy to understand.
Instead of hiding all the code in an openPopup Custom Hook, we hide only the implementation details and keep the core data — the popup’s title, content, and action — visible outside.
function QuestionPage() {
const [openPopup] = usePopup();
async function handleClick() {
const confirmed = await openPopup({
/*
* Popup title, content
*/
title: 'Insurance Question',
contents: <div>The agent will explain</div>,
});
if (confirmed) {
/*
* Popup action
*/
await submitQuestion();
}
}
async function submitQuestion(myExpert) {
await sendQuestionToMyExpert(questionValue, expert.id);
alert(`Asked ${myExpert.name} a question.`);
}
return <button onClick={handleClick}>Ask Question</button>;
}Now you can understand what kind of popup it is without reading the implementation details.
Declarative Programming
This development style — declaring:
Popup, I declare to you!
The title is "Insurance Question"
The content is "The expert will explain."
"And when the confirm button is clicked, send the question!"
When you declare this, the popup displays that content using pre-implemented details — this is called declarative programming.
The key feature of declarative programming is that you can quickly understand what a function does.
Implementation details are hidden so you don’t need to worry about them, and you can easily reuse by changing the “What”.
<Popup onSubmit={sendQuestion} onSuccess={navigateToHome} />The approach of writing each implementation detail individually instead of consolidating declaratively is called imperative programming.
<Popup>
<button
onClick={async () => {
const res = await register();
if (res.success) {
navigateToProfile();
}
}}
>
Submit
</button>
</Popup>Declarative programming also uses imperative code internally. Since all implementation details are exposed, customization is easy, but it takes time to read and is hard to reuse.
Q: Is declarative programming always better? A: No. You should use both approaches as appropriate.
Some might think declarative programming is better because it’s trendy, but each approach has its strengths.
React’s JSX syntax has the advantage of enabling declarative programming in HTML, but for passing “What” through props — passing such detailed content — imperative design is also needed.
Single Responsibility
We need to create functions with clear names that indicate they do one thing.
Let’s create an event handler function like the following. It’s the function name for when the question submit button is clicked in a form.
async function 〇〇〇〇() {
const termsAgreed = await getTermsAgreement();
if (!termsAgreed) {
await showTermsAgreementPopup();
}
await submitQuestion(questionValue);
alert('Your question has been registered');
}It checks if the user agreed to the terms and submits the question.
Since submitting the question is the core feature, handleSubmitQuestion seems good, right?
async function handleSubmitQuestion() {
const termsAgreed = await getTermsAgreement();
if (!termsAgreed) {
await showTermsAgreementPopup();
}
await submitQuestion(questionValue);
alert('Your question has been registered');
}No! That’s not good! The function name says “submit question,” but the implementation contains both terms check and question submission.
async function handleSubmitQuestion() {
/*
* Terms check
*/
const termsAgreed = await getTermsAgreement();
if (!termsAgreed) {
await showTermsAgreementPopup();
}
/*
* Question submission
*/
await submitQuestion(questionValue);
alert('Your question has been registered');
}Function names that don’t include all important aspects lead to a loss of trust in the code, because the code doesn’t behave as the reader expects.
After that, people stop trusting function names and start checking all the details.
What happens when a feature addition comes in?
async function handleSubmitQuestion() {
const termsAgreed = await getTermsAgreement();
if (!termsAgreed) {
await showTermsAgreementPopup();
}
await submitQuestion(questionValue);
alert('Your question has been registered');
const insuranceAgent = await getInsuranceAgent();
if (insuranceAgent !== null) {
await sendQuestionToAgent(questionValue);
alert(`Sent question to ${insuranceAgent.name}.`);
}
}The function gets even bigger, and handleSubmitQuestion ends up doing 2 additional roles beyond question submission.
We’ve all done it — just adding features to an existing function.
When this kind of feature addition repeats, the words our future selves use with others are:
You’d better not touch that code.
Let me fix it for now.
Let’s refactor so each function has a single role with a clear name.
async function handleSendQuestionToAgent() {
await sendQuestionToAgent(questionValue);
alert(`Sent question to ${agent.name}.`);
}
async function handleSubmitNewQuestion() {
await submitQuestion(questionValue);
alert('Your question has been registered');
}
async function handleTermsAgreementCheck() {
const termsAgreed = await getTermsAgreement();
if (!termsAgreed) {
await showTermsAgreementPopup();
}
}Split like this, you can read and use each one as needed.
Functional Components with Single Responsibility
Just as we split functions, React components can also be split by functionality.
<button
onClick={async () => {
log('Submit button clicked');
await openConfirm();
}}
/>There’s code that logs to the server when a button is clicked.
The unfortunate thing is that the button click function mixes a logging function with an API call.
<LogClick message="Submit button clicked">
<button onClick={openConfirm} />
</LogClick>Creating a LogClick component that automatically sends click logs when the button is clicked is a good refactoring approach.
This way, the button click function only needs to worry about the API call.
And there’s IntersectionObserver code that determines whether elements overlap.
const targetRef = useRef(null);
useEffect(() => {
const observer = new IntersectionObserver(([{ isIntersecting }]) => {
if (isIntersecting) {
fetchCats(nextPage);
}
});
return () => {
observer.unobserve(targetRef.current);
};
});
return <div ref={targetRef}>Load More</div>;It’s unfortunate that this Observer code’s implementation details and the API call are mixed together.
This can be separated by creating a higher-level IntersectionArea component that separates the Intersection detail code from the API call.
<IntersectionArea onImpression={() => fetchCats(nextPage)}>
<div ref={targetRef}>Load More</div>
</IntersectionArea>Abstraction
Core concepts can be extracted from logic through abstraction.
The following popup component code is implemented from scratch in detail.
/* Popup code implemented from scratch */
<div style={popupStyle}>
<button
onClick={async () => {
const res = await register();
if (res.success) {
navigateToProfile();
}
}}
>
Submit
</button>
</div>The following abstracts this popup code, keeping only the important concepts of submit action and success action.
/* Abstracted, keeping important concepts */
<Popup onSubmit={register} onSuccess={navigateToProfile} />Let’s also look at a function abstraction example.
The following fetches agent information and displays different labels based on the retrieved information.
/* Detailed implementation for getting agent label */
const planner = await fetchPlanner(plannerId);
const label = planner.new ? 'New Agent' : 'Connected Agent';The following abstracts all this implementation detail into a function called getPlannerLabel.
/* Abstracted with important concept in function name */
const label = await getPlannerLabel(plannerId);Like this, code such as components and functions can be refactored from concrete to somewhat abstract, or even more abstract.
Let’s look at one more example.
There’s concrete code that shows a Confirm when a button is clicked, and displays a specific message when the Confirm button is pressed.
/* Level 0 */
<Button onClick={showConfirm}>Submit</Button>;
{
isShowConfirm && (
<Confirm
onClick={() => {
showMessage('Success');
}}
/>
);
}The feature of showing a Confirm when the button is pressed has been abstracted into a ConfirmButton component.
You can use onConfirm to pass the action that fires when pressed.
/* Level 1 */
<ConfirmButton
onConfirm={() => {
showMessage('Success');
}}
>
Submit
</ConfirmButton>After using this code, you can abstract even further by just passing a message prop to display the desired message in the Confirm.
/* Level 2 */
<ConfirmButton message="Success">Submit</ConfirmButton>Going one step further, you can abstract all functionality into the ConfirmButton name.
/* Level 3 */
<ConfirmButton />There’s no right answer. Abstract as much as needed depending on the situation.
Here are actual review examples.
- Example 1
[Reviewer] Can’t you abstract all of this? Something like await moreAccurateLocation.request(). I don’t think the parent needs to know about opening the modal.
[Code Author] Good point. How about the name moreAccurateLocation.check()?
- Example 2
[Reviewer] UltraCallProgress and UltraCallComparison look similar. Can’t you abstract the common parts?
[Code Author] This part doesn’t have severe code duplication yet, and considering future flexibility, I think different parts will emerge, so I intentionally didn’t abstract it. (I’ve had quite a few painful experiences from abstracting too early.) I’ll think about it more.
When abstraction levels are mixed, code comprehension becomes difficult. When abstraction levels are mixed, it’s hard to understand at what level the overall code is written concretely.
When abstraction levels are mixed as follows, after seeing concretely written code, you might assume the highly abstracted code that follows is also concretely written.
<Title>Please rate.</Title>
<div>
{STARS.map(() => <Star />)}
</div>
<Reviews />
{rating !== 0 && (
<>
<Agreement />
<Button rating={rating} />
</>
)}So when you think the highly abstracted code is simple and look inside, you may find considerably complex code.
Writing this way disrupts thinking when reading code. It’s because you can’t determine at what level the code is concretely written.
<Title>Please rate.</Title>
<Stars />
<Reviews />
<AgreementButton show={rating !== 0}/>Writing at similar abstraction levels like this makes code much easier to understand.
Here we organized with high-level abstraction, but depending on the situation, organizing with low-level abstraction is also fine.
Action Items
We’ve studied the theory, so starting tomorrow, try applying Clean Code to your frontend code.
Here are the action items for that.
Don’t Be Afraid to Modify Existing Code
If you’re afraid of breaking code, you can’t create clean production code.
Using the excuse that PR File Changes become too many, we add code without breaking existing code.
Don’t be afraid — break existing code and architecture to write better code.
If you want to reduce PR File Changes, you can create a Mother Branch and add refactoring separately.
Practice Seeing the Big Picture
What was right then may not be right now.
Existing code may have been clean, but the code you added may have made it a mess.
The feature you added itself may be clean, but looking at the big picture, it may not be clean code.
Build Consensus with Your Team
There’s no right answer in code.
So when doing code reviews, there are times when you hesitate to leave comments about Clean Code.
The issue seems small right now so you hesitate to comment, but when these small consistency-breaking codes accumulate, the code becomes hard to maintain.
Consensus doesn’t form automatically. Explicit discussion time is needed.
Talk to each other about points you want to fix in code you built together, share the points you think are problems, and gather collective intelligence.
Then think about how to improve. You don’t need to find answers immediately. After sharing problems, take time to improve.
Write Documentation
Clean Code is an ambiguous concept. Writing it down makes it clear.
Write down your own principles about how this code might become risky in the future and how it can be improved.
Completed
Here’s a summary of the core React Clean Code concepts covered in this post:
- Cohesion: Consolidate code with the same purpose, but separate core data from implementation details
- Single Responsibility: Create functions with clear names that do one thing
- Abstraction: Extract core concepts, but keep abstraction levels consistent
This post is based on content presented at Toss’s SLASH 21 conference.
Was my blog helpful? Please leave a comment at the bottom. it will be a great help to me!
App promotion
Deku.Deku created the applications with Flutter.If you have interested, please try to download them for free.
