How to initiate Refactoring?
Tags: refactorit, rails, beginners, codequality
Published: 2022-05-24 19:08:44

## Introduction

4 years later, when I started programming I used to write code and just care about the results mainly **Its working How it's intended to be stance**. But as time passed I realized that whenever I want to add a new functionality within, it becomes nearly impossible for me. 

However Rails defaults enable us to stick with OOP principles and good practices, but as codebase grows you have to be a champion to hold up things properly!

So I have to compromise and search for escape routes. My bad. Being pro-active and habit of thinking to extend the genre is what I'm missing. By time I learnt from different experiences, features, other people's code and mainly from Google (How can I miss this!)

## Why we need Refactoring?

As I described above that it becomes hard to extend features, add new stuff but mainly you lose the control of your codebase. You become a slave of an unknown compelling force established within your app. Produced by your bad code and mis management.

Just think about big tech giants and social media apps, how code is being managed and scaled to reach a bigger audience. You'll get the answer for sure!

## Jumping into the Playground

Today I'm going to share my initial 5 steps approach to refactor a bad piece of code. It can be a starting point for those who don't have an idea about _Where to start?_. 

Lets start this!

As an example, we'll discuss this controller action in Rails which have code smells. It enables admin to assign user a specific subscription membership group.


![Bad piece](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/ckc924bsjb07p8gvrsh7.png)




...


###1: Dry run each line of code, understand meanings?

I have added comments against each line. See



![Code with comments](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/d54d1cwfl5j57nhqud0n.png)

...


###2: Identify responsibilities with Divide & Conquer rule

- Authorization check for user's role
- Query user from db and figure out user old membership and new group for further addition
- Update user with new attrs
- Send membership email
- Perform subscription expiry job
- Handle failure case of #update
- Handle failure case of authorization



###3: Extract methods by Single Responsibility Principle

> The single-responsibility principle (SRP) is a computer-programming principle that states that every module, class or function in a computer program should have responsibility over a single part of that program's functionality, and it should encapsulate that part.


![Single Responsibility Principle](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/gyhc2l5ob88ecy8r5335.png)


In accordance with responsibilites obtained in Step 2. I've added seperate methods. Each doing its own thing.
 
`check_user_role!` depicts authorization check, continue only if user is an *admin* otherwise redirect. The ! at the end describes that its going to follow-up immediately

`grab_user_and_associated_group` find user, get membership names, group and expiry date. Here I switched local variables with instance variables to manipulate them among micro actions. Provided the fact we can also use them inside views.

`user_params` prepares hash to pass inside update call

`update_user` updates user and call `forward_emails_then_set_job`. It has an `else` case to describe if something goes wrong.

`forward_email_then_set_job` calls mailer and job classes respectively. For sake of simplicity, I'm skipping discussion for them!

Notice about usage of redirect_back in this context, instead of using `redirect_to` with `request.referrer` You can find it [here](https://apidock.com/rails/v5.0.0.1/ActionController/Redirecting/redirect_back)


###4: Filter out performance gaps and fill them

If you notice down, there is a performance gap inside `forward_email_then_set_job`. The problem is `deliver_now` and `perform_now`. When we have a queue backend like Sidekiq in place, its essential to configure queues which can hold up jobs to perform one by one. `deliver_now` halts this priority queue and we can face the situations where jobs received at same time instant would be dropped, thus creating load on queue backend. 

For the solution, I've used `deliver_later` and `perform_later` which takes care of priority queue. Jobs are processed one by one, providing minimal drop chances and optimized queue backend



![Performance issue](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/ra7rmyns7g6vylnnok6m.png)


...

###5: Put all pieces and ask yourself *Is it improved*?



![Put all together](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/8ro8bkz5bjrz97za16t5.png)


The answer can be Yes or No. But the new version would be better than worst. A stable form!

Refactoring is costly if not done from the start. Its a habit towards self improvisation and better understanding of code.

I haven't came up with a perfect version. There are certain improvements we can do here. I would like to hear from you guys. Do Share and Like if you find it useful ..