A Grab-Bag of Ruby/Rails Tips

this video is a grab-bag of ten distinct tips now none of these tips quite warned at their own video but I'd wanted to make sure I mention them somewhere in the course and so here they are let's start off with our first example and I'm gonna start off in the specs so in this app we have posts and posts have a method on them to display them on a featured page and if the post is fresh and there's a low spam risk then we'll actually update the future flag but if the post is not fresh or if the user is a high spam risk we don't update the feature flag let's run these to get them queued up we are starting off green let's go to our post and here's that method I just talked about now this conditional is a compound conditional we have created at less than 10 days ago and and user has confirmed email I virtually always extract compound conditions like this into private methods I think it almost always makes the code better so let's do that now so right away I actually think this code has improved and the thing I like about this code is that it is now explicit about something that used to be implicit so before we had this compound conditional which represented whether or not the post was a good candidate for featuring and if I had to read this full conditional I'd have to think about why is this there but now I have this nicely named method that says why we're checking both these attributes I think whenever you can take something in your code that is implicit that becomes that's part of your knowledge or something you would have to ask a teammate and make it more explicit with a good name that's a big win but there's no reason to stop here I'd actually like to keep going and continue to make implicit things more explicit and I'm going to start by looking at the first side of this conditional which has created at less than ten days ago and what I'm really saying here is that the post is not too old to feature so let's extract another method cool nice little improvement and this other side here we're asking if the user has a confirmed to email that's not very clear the reason that we're checking if the user has confirmed email is because we want to make sure that this post is unlikely to be spam so again why don't we encode that in a better way cool now look how nicely our class is reading if this is a good candidate for featuring then feature it if you care more about what makes a good candidate for featuring you can look here and if you care more about any of those details about what makes something too old or what makes something a low spin risk you can look further down in the class also notice that all our methods are nice and short and small I love working with classes and systems that are composed of little tiny methods my favorite methods look like this they take no parameters and they're just one line now chances are if I were doing this refactoring on a real system I would probably stop here but what if the code already looked like this and I was about to add another condition here something like this at this point this is starting to look like a lot of logic to live right in the post model and so I would probably do something like extract a policy object so that might look like this and then I would copy over this not too old method and this low spam risk method into this new class that we just created I would need to do a little more work here but I think you can see the direction I'm going now we have this policy object whose responsibility is to answer when a post is a good candidate for being featured this is the more involved or factoring that I reach for when my conditionals end up with not just two branches but three branches by extracting this policy object testing gets a lot easier and it's nice to have all of the logic for determining when a post should be featured in one place it also lets me clear out these very specific private methods out of post and put them in a new class while I was refactoring I would probably leave this feature able post in here probably this class here private in the post eventually I would make this policy public and write unit tests for it so a quick review when you have compound conditionals extract them to private methods when you have implicit things try to make them explicit and if your conditionals continue to get hairy consider something like a policy object tip number two is to have a bin setup script this is something that I create in all of my rails applications and the job of the bin setup script is to get a new rails app up and running and you can see here the idea is to run it immediately after you cloned the codebase your bin setup script can start with this feel free to copy these things very basic things here like copying over the sample and giving the gems installed making sure everything looks good setting up the database and then this task which I'll talk about more in a second adding some hurich remotes whatever this can be totally specific for your setup the idea is any of the annoying things you have to do to get running throw them in here rather than a readme for example it's much nicer to just be able to run a script and then have to walk through steps in a readme one thing you're likely to experience if you do create a been set up script is that your app will migrate over time and the bin setup script will stop working but you won't notice it right away so when a new person joins your team or you set up the app on a new machine watch out for breakage here and get it fixed I don't think it's quite worth writing tests for the script which means that it will break over time but it's still worth having so I said I come back to this right here I like to also create a rake task for creating development seed data let's look at the example of that here's the task first thing to notice is that this task is only available in development and staging that's a nice idea for tasks that blow away the whole database and fill it with new things this will save you from shooting yourself in the foot you'll see there's another guard inside the actual task here I'll show you in a second and here is what your data creator might look like again we're protecting ourselves from foot shooting and then we have one high-level method called run that creates all of the data that we might want in our application to do useful development just like the bin setup script instead of giving people instructions to run in a readme that says hey also create a user create some teams create some plans create whatever just have a script that does it all I sometimes do write tests for tasks like this because like the bin setup script they are likely to break overtime if they're not tested there is one tension though where it's often nice to have your development data creator create something like hundreds of entries and that can be very slow to run every time you run your tests so there's a trade-off to balance tip number four is to watch out for comments now I think most people know the standard advice on comments which is if you have code that feels like it needs a comment see if you can improve the code until the comment becomes unnecessary for instance I will often make my code verbose give it very verbose and long names that sort of explain why I'm doing something weird if I felt like I needed a comment but there's a particular kind of comment that I think people consider an exception and creeps in even into good applications and that is the to-do comments here's an example right here that I took from a random open source project this is actually a twofer number one we have a commented out test don't leave commented out code in your apps and especially don't leave commented out tests if you feel the need to comment out of tests just delete it if you feel the need to comment out some code just delete it it has it if you ever need it you can get it back just blow it away this is like leaving trash in your kitchen but coming back to this to do comments to do's don't belong in code in my opinion to do is belong in whatever your issue management system is I don't think there's anyone in the world that gets in on Monday morning says I'm gonna knock out some of the random to do is scatter throughout our app if it to do is important you should promote it Intuit github issue or a Trello card or whatever use to manage your work and that way it can be prioritized and not forgotten and just to back up my point a little bit on this the history of this to do it has been here for over a year in this application untouched to do is just hang around delete them or promote them into real issues that said I think there is one kind of comment that's alright I took this from the discourse project this I think is not a terrible comment it's talking about how the fact that yes something weird is happening right here but it's much more efficient than active record so leave it alone this part is performance critical I think it would be a little tricky to get this comment into the code although I might do something like this not sure though I might just leave his comment as is tip number five is to organize the methods in your class in descending order of abstraction that looks like this this is a class for making hard-boiled eggs and if you want to make hard-boiled eggs you prepare the pot you add the eggs and then you wait until they're cooked notice how all three method calls here are at a very high level of abstraction that is what I'm always shooting for in my public methods I want someone to be able to open a class and get a super high level overview of how the class works now if I want or need to I can dive down into a deeper level of abstraction and that's all hidden behind this private keyword but let's look we have prepare pot and so the way you prepare a pot is you locate it you fill it and then you place it on the stove notice even though we've descended below the private keyword we are still doing a high-level description of a slightly lower level step so I've descended a level of abstraction from what's up here but I'm still trying to keep it fairly high level let's scroll down a little further here we've dropped down another level of abstraction to locate in the pot filling with water and placing it on the stove and at the very bottom we get down to the nitty gritty of something like is pot like let's jump back up to the top of the class notice how this reads high-level three steps this is the first step this is the second step this is the third step so you prepare add eggs wait until cook prepare at eggs wait until clicked then I dive down into prepare pots methods locate pot fill pot with water place pot on stove and then if there were methods and add eggs and wait until cooked they would appear with any threes the level of abstraction of my methods moves smoothly from high to low as you read down the class this is great because if you don't care about the details you can stay at the top and as you need to get more into the nitty-gritty you just keep scrolling imagine I ignored this and moved this is potluck method to the top do you see how out of place this looks now we've gone from super high level and then we go below the private keyword and all of a sudden we're talking about responds to and size and things like that we've lost that nice consistency that this class used to have I'm going to undo that something to notice about this it can be tricky to know where to put a method at what level of abstraction this is especially hard as your class gets more methods and my answer to that is don't put more methods in your classes if you're having a hard time organizing the methods in your class by level of abstraction there are probably too many methods in your class one final nice benefit here notice that I could basically draw a line at any point and extract everything below that point into a new object and have it makes sense that's pretty awesome I could at any point decide that there's too much going on in this class and extract things I could even extract each of these methods into their own object and the class would still make sense there's very little out have to rearrange this will give me a nice clean diff when I open up a pull request tip number six is to understand how the tap method works I'm gonna open up some specs first and run these don't worry about this quite yet let's just hop over here okay super simple example we are taking a user updating some attributes calling save and then returning that user now we can't just remove this line because user dot save returns true as opposed to the user so we need to explicitly return the at user here one little micro optimization you can make is to use the tap method still works tap takes whatever object you call it on and passes it in as a parameter to the block and then returns it so this at user becomes just this local block user we can mutate it and then save it and at the end tap evaluates to the value of user just a small tip but sometimes very handy notice that I never reached for the return keyword that's because I like to rely on the fact that Ruby will automatically return the last thing that was evaluated in a method however there is one place I do like return and that is in early returns here's another example pulled from discourse which is right here this guard clause so if you want to see if someone has posted too much in a topic we have some work to do however we don't want to apply that to staff and non new members so we can bail out early here I actually like having guard clauses like this at the start of my method because it makes it easy to ignore the rest of the method if this thing applies notice that if this code didn't use this one-liner in this way we would need to do something like this this isn't too bad but imagine needing to add another condition where this doesn't apply suddenly we have another nesting here this is getting uglier and uglier however if we stick with this guard Clause approach we can just keep adding conditions without indenting the rest of the code I'm pretty into this tip number seven is to prefer the exception raising variance over silent failing so this user class has a deactivate method on it where we set the active flag to false and then save it and then email the user a deactivation notice but the thing to notice is that save here could totally fail and if it failed they would just return false that false will get discarded and then we would incorrectly email the user a deactivation notice that's why I'd prefer to do things like save bang if this thing fails when it shouldn't I want to know about it I tend to prefer my failures to be noisy rather than silent because of this I prefer the save bang method update bang method create bang method etc whenever I can have an exception appear as opposed to failing silently I'm into that tip number nine is don't use a control couple so what does a control couple look like a control couple is something you pass into a method to tell it which branch to take so here's some code where a comment has a post method and you can pass a flag about notifying admins so if I just call comment that post it will notify the admins and if I pass this false boolean in it won't notify the admins this parameter here should notify admin is called a control couple and it's called a control couple because the code here is more coupled to the method it's calling than it needs to be specifically the code here calling comment a post and passing false knows what it wants to happen rather than just rely on post to do the right thing the code calling comment at post and passing false knows it doesn't want to notify admins and it knows that there's a conditional inside that looks at the parameter passes in a better way would be to have two methods this one notifies admins and this one doesn't since the calling code already knows what path it wants it should just have it the option to call a method that takes that path rather than controlling the execution with a boolean notice that both of these method calls don't have a parameter the parameter coupling is gone that means this code is a bit better than it used to be now we can change the inside of post and the inside of post without notifying admins without changing the calling code but up here if you want to change the method by which we specify that admins it shouldn't be notified the calling code needs to change and the code that's being called needs to change at the same time that coupling is high we don't like it this is better approach one example from real life it used to be that you could call save false in rails to skip validations later the team decided that they wanted to have it looked like this they wanted to pass a hash as opposed to a single boolean because they wanted to be able to pass additional options in to save also because say if false is not very obviously clear what's happening when they made the change from save false to save validate false all of our client code had to change everywhere you were calling say false you now needed to change that parameter that was the cost of the coupling introduced by this boolean parameter here now imagine it had instead been save and stay without validation now rails can change the contents of those methods all they want to change all the internals as much as they want and none of the client code has to change that is the benefit of lower coupling my final tip tip number 10 is to not use instance variables in partials every so often I like to grep my rails app for violations of this here I'm using a G known as the silver searcher which is a faster grep replacement and I'm looking for the @ symbol in any file in the views directory that starts with an underscore looks like I do have a violation in this app and it looks like this I'm in the underscore forum' partial and I am referencing at user now you don't want to reference instance variables in your partials because it couples the partial to that particular controller that sets up that instance variable you want to be able to reuse this partial wherever you want to display its contents without worrying about what controller render the view originally fortunately the fix for this is quite simple I'll go to where I'm calling this partial which is right here and I'll change it to look like this now I'm passing in the at user as a user local going back to my partial I can just reference a normal user like this and again we picked up a nice win this partial is now agnostic about where you just come from which means I'm more likely to be able to use it from other places and not worry about what controller set up the user variable originally so that's it that's my 10 tips I hope it was useful and I'll see you next time

Channel: Ben Orenstein

Views: 5,935

Rating: 4.9819002 out of 5

Keywords: ruby, rails, refactoring, tips

Id: 0rsilnpU1DU

Length: 18min 50sec (1130 seconds)

Published: Wed Oct 18 2017