00:00:07.560
All right. I just have a quick question. This is a Ruby conference and not a Rails conference, but I'm just curious how many here have played with Rails? How many do Rails programming professionally? This is almost a Rails conference, anyway. We've been asked to do a live code review of a real Rails application. I'd like to thank Joel for providing this application for us to look at. It's always kind of frightening to have a large number of people look over your code and have someone else go over it and point out what could be done better. We really appreciate that. Should we just hop into it? Yeah? All right, so let's first pull up the Community controller.
00:00:26.599
To tell you the truth, we're not entirely sure what this application even does because we couldn't run it — we didn't have PostgreSQL set up. So we're not looking at it from the perspective of how this application should have been written to do what it needs to do better. We're just examining it from the standpoint that certain snippets of code could probably be done differently to fit better with Ruby and Rails idioms. One thing that struck us is a pattern that shows up a lot where something is assigned. If not, the first nitpick is to use 'unless.' This is a more Ruby-style idiom. Instead of saying 'if not,' you can say 'unless.' Even better, since we're just checking to see if the current tab is null or not, we can eliminate that whole second line.
00:01:04.519
Another area we're looking at is the check for the offset. If it isn't nil, there's a call to 'to_i.' Otherwise, it sets it to zero. Fortunately, 'nil.to_i' is zero, which simplifies that code. This one is a bit hairier to condense into one line, and you don't always want to go for one-liners anyway. Ultimately, all of this should really be pulled into the 'get_communities' method on the Community controller. You would assign all the necessary parameters — like 'params[:offset]' and 'params[:limit]' — and then let the 'communities' method determine what to do in case they are nil or whatever. There's a part in Ken Beck's 'Smalltalk Best Practice Patterns' where he gives a tip on how to approach refactoring your methods and determining where to decompose methods. His tip is that all things happening in a given method should be at the same level of abstraction.
00:02:19.720
In this case, we have the notion of getting communities, which is the higher-level operation, and then we have all these little implementation details about how to get those communities, like setting the offset and the limit. In the controller, all you want are the higher-level symbols of what's happening; thus, it should really just be about getting communities. You want to make the data available for that so you're not mixing those two aspects and context-switching between lower and higher-level concepts.
00:03:08.560
Does anybody know the difference between 'to_i' and 'Integer'? In the context of what we were saying, when we were discussing handling nil offsets, using 'to_i' would return zero when the passed string is empty, while using 'Integer' would raise an exception if that string can't genuinely represent a number. In cases where it matters that you're strictly expecting either nil or an integer, using 'Integer' is the preferable choice because an exception is raised rather than just defaulting to zero. This difference can lead to subtle bugs if you're not careful.
00:04:21.919
In general, when handling business logic, it probably depends on what the rules are in your application. I would argue one shouldn't be coercing values in the controller directly; that logic should be implemented in the model. The story here is that this code structure could be refactored in a more logical way, but given its current state, there are certainly better alternatives.
00:05:14.160
In fact, there's a lot of value in keeping controllers slim. When you have a number of nested 'if' statements, it may indicate a failure to utilize polymorphism, which can signal a design smell. You can express behaviors better by encapsulating concerns in object behavior rather than piling more procedural code with all the 'if' statements.
00:05:59.520
Returning to the code, when dealing with booleans and error handling, it's worth considering whether exceptions could handle this logic better. When there are multiple conditions involved interacting with booleans, it obscures higher-level understanding of what is going on. This higher-level operation should ideally determine whether an issue arises and raise an appropriate exception instead of maintaining silhouette states within procedural code.
00:07:02.039
The best practices for organizing Rails applications emphasize designing robust models because that leads to thin controllers. I find that a controller action should contain a minimal number of lines — ideally five at most — and serve as a clear declaration of intent rather than being cluttered with implementation details. For example, an action should say 'banish user if not allowed,' which provides clarity rather than digging through various method calls to deduce what that logically means.
00:08:33.360
It's essential to understand that when dealing with models, you can also encapsulate multiple model interactions in a 'super model' that doesn't have to inherit from Active Record if that simplifies the logic. By abstracting complex relationships into a proper encapsulated model, your controller can stay simple, allowing for clear API calls and concise testing methods.
00:09:51.840
Additionally, if the structure of your controllers is coherent — with clear design principles established — testing becomes streamlined. By shifting logic into models, one can leverage unit tests which are generally more flexible than large functional tests that can be brittle. Your testing leads to clearer expectations of behavior, which allows for higher quality code.
00:11:03.959
It's vital during development to hold a cohesive relationship between controllers and models, allowing for easily extendible and reusable functionalities. This leads to the realization that well-structured code not only benefits usability but also maintains integrity as further developments occur.
00:12:02.200
Concerning view layer interaction, particularly when employing ERB, you should strive to maintain the clarity of intent. Instead of performing intricate logic within your views, you'd better serve the application by isolating responsibility within helpers. Creating blocks that encapsulate logical conditions is a straightforward way to elevate clarity while also optimizing for DRY practices.
00:12:50.400
The function to generate conditional helpers retains the benefits of abstraction while offering a clean solution to avoid cluttering your templates with repeated input. By including basic wrappers for ubiquitous structures, the readability of your views is greatly enhanced, and any conditional display logic becomes manageable.
00:14:16.280
To summarize, effective application structure revolves around succinct controllers, expressive models, and evaluative views. This fusion is fundamental to optimizing Rails app development for long-term clarity and maintainability. By continually applying these principles throughout a project's lifecycle, you set a solid foundation for both current and future developers.
00:15:41.480
Your groundwork in ensuring adherence to Rails conventions bolsters the advantages present in a robust framework. Substantial opportunities for code reuse stem from this coherent approach and serve the community as we endeavor to share knowledge and improvement practices.
00:17:09.600
As we conclude this discussion, I'd like to open the floor for any final thoughts or questions. Remember, as you implement these evolving practices and philosophies, working fluidly across your application's boundaries while adhering to these principles will always lead toward better coding experiences.
