Last active
December 30, 2015 23:39
Revisions
-
codeodor revised this gist
Dec 10, 2013 . 1 changed file with 2 additions and 2 deletions.There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode charactersOriginal file line number Diff line number Diff line change @@ -12,7 +12,7 @@ https://github.com/rails/rails/pull/12065 I linked to the PR instead of the commits so you all can read the discussion easily. Another interesting one: a lot of people use the pattern `[option, option2, option3].include?(option1)` as a cleaner way of doing `option1 == option1 || option1 == option2 || option1 == option3`. But if it's in a method that gets called often, it results in poor performance due to all the array allocations: https://github.com/rails/rails/pull/11955/commits and specifically this commit: https://github.com/SamSaffron/rails/commit/eb9f3e0a164be1299b5d5233f2e88d5dad654357 @@ -23,7 +23,7 @@ Sorry for all the Sam Saffron commits. :smile: I was looking for one in particul https://github.com/rails/rails/pull/12188/files As you can see in the discussion surrounding the issue, it introduced some bugs (due to interaction w/ protected_attributes). IMO, part of the cause is the use of `@instance` vars everywhere -- it causes trouble, not the least of which is because it makes it hard to figure out what's defined where, and especially so in a code base like Rails, which has lots of modules included that rely on `@vars` defined elsewhere. I prefer to wrap them in methods, and it always helps if you need to refactor or change behavior of something surrounding the `@var` since you don't have to track down all the places it is referenced. In any case, sorry I won't be able to make it tonight, but I hope the code referenced in this gist will spur the discussion. -
codeodor revised this gist
Dec 10, 2013 . 1 changed file with 1 addition and 1 deletion.There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode charactersOriginal file line number Diff line number Diff line change @@ -25,6 +25,6 @@ https://github.com/rails/rails/pull/12188/files As you can see in the discussion surrounding the issue, it introduced some bugs (due to interaction w/ protected_attributes). IMO, part of the cause is the use of @instance vars everywhere -- it causes trouble, not the least of which is because it makes it hard to figure out what's defined where, and especially so in a code base like Rails, which has lots of modules included that rely on @vars defined elsewhere. I prefer to use methods, and it always helps if you need to refactor or change behavior of something surrounding the @var since you don't have to track down all the places it is referenced. In any case, sorry I won't be able to make it tonight, but I hope the code referenced in this gist will spur the discussion. Sam -
codeodor created this gist
Dec 10, 2013 .There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode charactersOriginal file line number Diff line number Diff line change @@ -0,0 +1,30 @@ I won't make it tonight because while I thought I'd be recovered from illness by now, I'm still a little sick and don't want to infect the entire Houston Ruby community with a nasty virus! But I had some ideas for code, so if you need some, here they are: These commits to Rails might seem like bad code because they take something elegant and turn it into longer uglier code, but the performance gains are nice! https://github.com/rails/rails/pull/12185 and https://github.com/rails/rails/pull/12065 I linked to the PR instead of the commits so you all can read the discussion easily. Another interesting one: a lot of people use the pattern [option, option2, option3].include?(option1) as a cleaner way of doing option1 == option1 || option1 == option2 || option1 == option3. But if it's in a method that gets called often, it results in poor performance due to all the array allocations: https://github.com/rails/rails/pull/11955/commits and specifically this commit: https://github.com/SamSaffron/rails/commit/eb9f3e0a164be1299b5d5233f2e88d5dad654357 I prefer the nice pattern we've developed (using the array), but it's good to be cognizant of its pitfalls when it's used excessively (or called 1000s of times in a request) Sorry for all the Sam Saffron commits. :smile: I was looking for one in particular that I'd run into trouble with in the past, and came across those when I was searching. Here's the one I think I was trying to find: https://github.com/rails/rails/pull/12188/files As you can see in the discussion surrounding the issue, it introduced some bugs (due to interaction w/ protected_attributes). IMO, part of the cause is the use of @instance vars everywhere -- it causes trouble, not the least of which is because it makes it hard to figure out what's defined where, and especially so in a code base like Rails, which has lots of modules included that rely on @vars defined elsewhere. I prefer to use methods, and it always helps if you need to refactor or change behavior of something surrounding the @var since you don't have to track down all the places it is referenced. In any case, sorry I won't be able to make it tonight, but I hope the code referenced in this email will spur the discussion. Sam