You know that feeling when you're in the mood to crack on some code and do a 'brain dump' of ideas and code it? I'm sure everyone does, and it's perfectly fine. But sometimes, we forget about the initial brain dump and carry on with our lives, jobs, and continue coding, adding more complexity to the codebase. Until one day, it hits you and you figure out that you need to revisit 'an old friend' the old code and refactor it to make it polished, useful, easy, and maybe reusable for the future. Good thinking! This happens to me all the time – It feels like I'm constantly learning and growing in my quest to achieve better refactoring in my code.
🏁 Initial code
In a project, I needed to set a limitation on how many times a user could create a post. The application required only 3 posts per user (talk about exclusivity, huh?). Here's my initial code:
class PostsController < ApplicationController
before_action :check_limited_posts, if: :signed_in?, only: :new
private
def check_limited_posts
limited_posts = 3
return unless current_user.posts.size == limited_posts
redirect_to posts_path, notice: 'Currently we only offer a maximum of 3 posts.'
end
end
As you can see, there's nothing complex here, and the code is fine – it doesn't 'have to' change, right? But, we developers should always follow best practices, and refactoring is a fundamental one that we should always strive for (like superheroes of clean code!).
🧹 Refactoring
So, I rolled up my sleeves and got to work on refactoring the code to make it more professional and reusable, and here is what I want to do:
Extract the maximum number of allowed posts into a constant (because, who knows, we might be feeling more generous one day).
Use a more descriptive name for the constant. And as you, naming is not easy.
Use a more descriptive name for the before_action
method.
class PostsController < ApplicationController
MAX_POSTS_ALLOWED = 3
before_action :check_max_posts_allowed, if: :signed_in?, only: :new
private
def check_max_posts_allowed
if current_user.posts.size >= MAX_POSTS_ALLOWED
redirect_to posts_path,
notice: "Currently we only offer a maximum of #{MAX_POSTS_ALLOWED} post(s)."
end
end
And voila! The check_max_posts_allowed the method is now more reusable, and it even has an easy name to remember (like a cool band name or a secret code). The maximum number of allowed posts is now defined as a constant, making it easier to modify in the future (perfect for those ever-changing project requirements). The method and constant names are more descriptive, making the code more readable and understandable, like a well-written novel.
🗞 Conclusion
Refactoring may seem like a trivial task, but it is an essential part of the development process. By following best practices and continuously refining our code, we can create more maintainable, reusable, and efficient applications. It's like decluttering our digital workspace, so our future selves will thank us!
So, the next time you find yourself looking at your old code, remember the power of refactoring. Embrace the challenge, and let's keep striving to make our code better, cleaner, and more professional. After all, we're the superheroes of clean code, right?
😉 One more thing
But we're not done yet! Let's make our code even more modular and maintainable by moving the validation logic to a separate service object and using localization for our notice text. Wow, fancy stuff 😎
Step 1: Move validation logic to a service object
Create a new service object called PostValidator in the app/services
directory:
class PostValidator MAX_POSTS_ALLOWED = 3
def self.check_max_posts_allowed(user)
user.posts.size >= MAX_POSTS_ALLOWED
end
end
Now, update the PostsController
to use the new PostValidator
service object:
class PostsController < ApplicationController
before_action :check_max_posts_allowed, if: :signed_in?, only: :new
private
def check_max_posts_allowed
if PostValidator.check_max_posts_allowed(current_user)
redirect_to posts_path, notice: "Currently we only offer a maximum of #{PostValidator::MAX_POSTS_ALLOWED} post(s)."
end
end
end
Step 2: Use localization for the notice text
Update the views.en.yml
file in the config/locales
folder:
en:
posts:
new:
max_posts_notice: "Currently we only offer a maximum of %{max_posts} post(s)."
Notice that I'm using a views file which is different from the default rails-generated en.yml
file. The views file focuses on the actions new, index, show, ...
for each object post
.
Next, update the PostsController
to use the notice text from the localization file:
class PostsController < ApplicationController
before_action :check_max_posts_allowed, if: :signed_in?, only: :new
private
def check_max_posts_allowed
if PostValidator.check_max_posts_allowed(current_user)
redirect_to posts_path, notice: t('.posts.max_posts_notice', max_posts: PostValidator::MAX_POSTS_ALLOWED)
end
end
end
Notice the .
in t('.posts.max_posts_notice',
it is a reference to the object posts within views file. If you forget it you will get translation missing: en.max_posts_notice
error.
And there you have it! Our code is now more modular, maintainable, and ready for any future changes. With these simple refactoring techniques, we've improved the readability and organization of our Ruby on Rails code, all while having a bit of fun along the way. Yuppy 👏🏼🤖
And as always, Happy Coding 😀 💻
Top comments (2)
I think the most important thing is that you fixed the bug from the original code: if by some race condition user was able to have fourth post, they were able to keep adding more and more, because equality was checked instead of
gte
😛But nice example of making the code much better in general.
Haha, you caught that! 😄 You're absolutely right, using the
>=
operator does help in handling those sneaky race conditions and keeps our code more robust. Thanks for pointing it out, and I'm glad you found the improvement valuable! Let's keep squashing those bugs and making our code even better! 🚀💪