5 ms·

My problem with Ruby is not so much the syntax. I kinda like this one-line-function feature, and I enjoyed it when using in C#. My problem is the fact Ruby pra

by whstl 3y ago

My problem with Ruby is not so much the syntax. I kinda like this one-line-function feature, and I enjoyed it when using in C#. My problem is the fact Ruby practitioners have a tendency ABUSE the usage of one-line-methods with LOTS of side-effects. So it's not like Haskell one-line-methods. Things that could be a function with 5 or 6 lines can become a class with as many methods. And instead of local variables, you now have to use instance variables (class fields). For example, this is common in lots of codebases: class UserCreator def initialize(email) @email = email end def create_user create_user_object assign_admin_role assign_invite_permission call_invite_template end private attr_reader :email, :user def create_user_object @user = User.create(email) end def assign_admin_role user.roles << find_admin_role end def assign_invite_permission user.permissions << find_invite_permission end def call_invite_template find_invite_template.call(user: user) end def find_admin_role Roles.find_by(name: 'admin') end def find_invite_permission Permissions.find_by(name: 'invite') end def find_invite_template Templates.find_by(name: 'invite') end end Sorry but this is not readable nor reasonable. I encountered this in about 4 companies so far. Only one Ruby company I worked didn't do it. That was the company that actually had a very maintanable backend.

ht85 3y ago
I call it "assembly ruby", where registers start with "@" and opcodes are defined with "def". So dreadful.
cratermoon 3y ago
https://thomascothran.tech/2023/11/readability/ https://thomascothran.tech/2023/11/readability/
JasserInicide 3y ago
What is wrong with this example exactly? This is what you want in a service object.
- vidarh 3y ago
  It's reducing readability by introducing extra methods you have to chase down where the method content is just as clear as the method names themselves. If there are multiple clients that might use those individual methods then maybe some of it is excusable. Personally I'd refuse to accept a commit like that if reviewing that code.
  - ljm 3y ago
    I notice this pattern often appears as a result of taking rubocop’s default settings as gospel. It pretty much forces you into pointless abstraction by saying a method should be less than 5 lines, or a class less than 100, or whatever, even though such abstraction would offer no actual value. Oftentimes when I suggest to change a rubocop default it leads to epic arguments as if you’re desecrating the bible. In fact, it’s just silly to not apply critical thinking to the quality of your toolchain, and that includes questioning the decisions of your linter’s maintainers.
    - vidarh 3y ago
      In this case, though, it's not even producing shorter code. It's producing longer, less readable code.
- lstamour 3y ago
  The methods that start with “call_” and “find_” are particularly redundant, because they aren’t any simpler than the code they contain. Technically true of all the methods given but the rest could be excused as explaining less readable syntax or for future expansion if parameters change. Modules should be deep, not shallow, is one design approach. This half fulfills that because the methods are private, but it’s also true that the method names are effectively comments yet they obscure the natural reading order for the code. See John Ousterhout’s A Philosophy of Software Design for more on this idea of deep vs shallow modules. (Edit to add: another way of justifying the above service design pattern is to suggest that the code is DRY because you’ve extracted common sections to methods. This is taking DRY to an illogical extreme. You _should_ repeat yourself if your method calls are simple enough, and if you actually do need to repeat some logic in multiple places requiring a private method, you might find a better place to put it - for example, a filter method.)
vidarh 3y ago
This is much more of a Rails thing than a Ruby thing, and as someone who loves Ruby, I agree - when I come across code like that it is cringe-inducing.
silasb 3y ago
I don't mind this style, it's encouraged by dry-monads with the do-notation. The big concern that I have with this code is it's missing a service level transaction.
- whstl 3y ago
  I don’t have a problem with small functions in funcional code, including code built around dry-monad, but this code is definitely not in this category :/
pkkm 3y ago
This is just the Clean Code style. If you read the book (or an article with examples [0]), you'll see that it advocates exactly this kind of thing: refactoring functions that are "long" and "unreadable" because they have a dozen or two dozen lines into a "clean" and "maintainable" web of tiny functions calling one another. Personally, I find this style very mentally straining because I have to constantly jump around and keep track of a huge function call graph in my head; it's almost as bad as beginner code with a thousand lines per function. The popularity of fads like this in the Ruby community is one of the reasons I'm glad I switched to Python. [0] https://qntm.org/clean https://qntm.org/clean
- whstl 3y ago
  “Mentally straining” is a good way of putting it. It also makes debugging much harder, as you have to keep track of instance variables in your head.
- ht85 3y ago
  If only functions could take arguments... Doesn't that already look 100x better? class UserCreator def create_user(email) user = create_user_object(email) assign_admin_role(user) assign_invite_permission(user) call_invite_template(user) end private def create_user_object(email) User.create(email) end def assign_admin_role(user) user.roles << find_admin_role end def assign_invite_permission(user) user.permissions << find_invite_permission end def call_invite_template(user) find_invite_template.call(user: user) end def find_admin_role Roles.find_by(name: 'admin') end def find_invite_permission Permissions.find_by(name: 'invite') end def find_invite_template Templates.find_by(name: 'invite') end end
  - whstl 3y ago
    Slightly less worse, but the original function that I inputted into ChatGPT to generate the code from my message was this: def create_user(email) user = User.create(email: email) user.roles << Roles.find_by(name: 'admin') user.permissions << Permissions.find_by(name: 'invite') Templates.find_by(name: 'invite').call(user: user) end IMO this 4 line function is significantly better in terms of clarity, readability, and it avoids unnecessary state. Testability and encapsulation are the same. (I would argue that the encapsulation is better with a function, since encapsulation is way too easy to break in Ruby, but hey, that's me)
michaelteter 3y ago
While I personally prefer more functional (non-oop) approaches, the example you show is positive to me. If the functions are well named, you can read up to the "private" line and stop reading. You now know what this thing does, and you've not had to complicate your thought about HOW it does it. If you need to know how one of the primary actions is performed, you can jump to that source with a simple keypress. With some settings, you can simply hover to see the function expanded. What's more, writing tests - 100% coverage - for this example is trivial and clean. If you want to put the actual lines all in the main function, your tests will be a lot uglier and will involve more lines of test code.
- whstl 3y ago
  The problem is that I need to know HOW this class does something, especially when debugging. The unnecessary branching makes me need to keep an eye on the stack, and a lot of extra information in my head. The additional state (in this case @email and @user) needs to be tracked from a distance. Local variables are 100x better. If there's any mutation, I must watch out during debugging two or three levels deep. Often there are more variables than that. If I had the "role" and the "permission" as a parameter, I would need two extra instance variables. With a regular function this is just two arguments, a couple lines away from the usage site. The original code I wrote into ChatGPT to generate the code above shows me the what and the how with much more clarity: def create_user(email) user = User.create(email) user.roles << Roles.find_by(name: 'admin') user.permissions << Permissions.find_by(name: 'invite') Templates.find_by(name: 'invite').call(user: user) end
  - fknorangesite 3y ago
    Yes, but that's because the example we were given in this thread is just a toy strawman in that each of the methods are just simple one-liners anyway. So yeah, given this literal code sample, I agree the private methods are extraneous. The point of this pattern is to extract each operation into its own described-by-name method because they're each longer and more complicated than just a `<<` operator or whatever.

by whstl 3y ago

ht85 3y ago

I call it "assembly ruby", where registers start with "@" and opcodes are defined with "def". So dreadful.

cratermoon 3y ago

https://thomascothran.tech/2023/11/readability/ https://thomascothran.tech/2023/11/readability/

JasserInicide 3y ago

What is wrong with this example exactly? This is what you want in a service object.

vidarh 3y ago
It's reducing readability by introducing extra methods you have to chase down where the method content is just as clear as the method names themselves. If there are multiple clients that might use those individual methods then maybe some of it is excusable. Personally I'd refuse to accept a commit like that if reviewing that code.
- ljm 3y ago
  I notice this pattern often appears as a result of taking rubocop’s default settings as gospel. It pretty much forces you into pointless abstraction by saying a method should be less than 5 lines, or a class less than 100, or whatever, even though such abstraction would offer no actual value. Oftentimes when I suggest to change a rubocop default it leads to epic arguments as if you’re desecrating the bible. In fact, it’s just silly to not apply critical thinking to the quality of your toolchain, and that includes questioning the decisions of your linter’s maintainers.
  - vidarh 3y ago
    In this case, though, it's not even producing shorter code. It's producing longer, less readable code.
lstamour 3y ago
The methods that start with “call_” and “find_” are particularly redundant, because they aren’t any simpler than the code they contain. Technically true of all the methods given but the rest could be excused as explaining less readable syntax or for future expansion if parameters change. Modules should be deep, not shallow, is one design approach. This half fulfills that because the methods are private, but it’s also true that the method names are effectively comments yet they obscure the natural reading order for the code. See John Ousterhout’s A Philosophy of Software Design for more on this idea of deep vs shallow modules. (Edit to add: another way of justifying the above service design pattern is to suggest that the code is DRY because you’ve extracted common sections to methods. This is taking DRY to an illogical extreme. You _should_ repeat yourself if your method calls are simple enough, and if you actually do need to repeat some logic in multiple places requiring a private method, you might find a better place to put it - for example, a filter method.)

vidarh 3y ago

This is much more of a Rails thing than a Ruby thing, and as someone who loves Ruby, I agree - when I come across code like that it is cringe-inducing.

silasb 3y ago

I don't mind this style, it's encouraged by dry-monads with the do-notation. The big concern that I have with this code is it's missing a service level transaction.

whstl 3y ago
I don’t have a problem with small functions in funcional code, including code built around dry-monad, but this code is definitely not in this category :/

pkkm 3y ago

This is just the Clean Code style. If you read the book (or an article with examples [0]), you'll see that it advocates exactly this kind of thing: refactoring functions that are "long" and "unreadable" because they have a dozen or two dozen lines into a "clean" and "maintainable" web of tiny functions calling one another. Personally, I find this style very mentally straining because I have to constantly jump around and keep track of a huge function call graph in my head; it's almost as bad as beginner code with a thousand lines per function. The popularity of fads like this in the Ruby community is one of the reasons I'm glad I switched to Python. [0] https://qntm.org/clean https://qntm.org/clean

whstl 3y ago
“Mentally straining” is a good way of putting it. It also makes debugging much harder, as you have to keep track of instance variables in your head.
ht85 3y ago
If only functions could take arguments... Doesn't that already look 100x better? class UserCreator def create_user(email) user = create_user_object(email) assign_admin_role(user) assign_invite_permission(user) call_invite_template(user) end private def create_user_object(email) User.create(email) end def assign_admin_role(user) user.roles << find_admin_role end def assign_invite_permission(user) user.permissions << find_invite_permission end def call_invite_template(user) find_invite_template.call(user: user) end def find_admin_role Roles.find_by(name: 'admin') end def find_invite_permission Permissions.find_by(name: 'invite') end def find_invite_template Templates.find_by(name: 'invite') end end
- whstl 3y ago
  Slightly less worse, but the original function that I inputted into ChatGPT to generate the code from my message was this: def create_user(email) user = User.create(email: email) user.roles << Roles.find_by(name: 'admin') user.permissions << Permissions.find_by(name: 'invite') Templates.find_by(name: 'invite').call(user: user) end IMO this 4 line function is significantly better in terms of clarity, readability, and it avoids unnecessary state. Testability and encapsulation are the same. (I would argue that the encapsulation is better with a function, since encapsulation is way too easy to break in Ruby, but hey, that's me)

michaelteter 3y ago

While I personally prefer more functional (non-oop) approaches, the example you show is positive to me. If the functions are well named, you can read up to the "private" line and stop reading. You now know what this thing does, and you've not had to complicate your thought about HOW it does it. If you need to know how one of the primary actions is performed, you can jump to that source with a simple keypress. With some settings, you can simply hover to see the function expanded. What's more, writing tests - 100% coverage - for this example is trivial and clean. If you want to put the actual lines all in the main function, your tests will be a lot uglier and will involve more lines of test code.

whstl 3y ago
The problem is that I need to know HOW this class does something, especially when debugging. The unnecessary branching makes me need to keep an eye on the stack, and a lot of extra information in my head. The additional state (in this case @email and @user) needs to be tracked from a distance. Local variables are 100x better. If there's any mutation, I must watch out during debugging two or three levels deep. Often there are more variables than that. If I had the "role" and the "permission" as a parameter, I would need two extra instance variables. With a regular function this is just two arguments, a couple lines away from the usage site. The original code I wrote into ChatGPT to generate the code above shows me the what and the how with much more clarity: def create_user(email) user = User.create(email) user.roles << Roles.find_by(name: 'admin') user.permissions << Permissions.find_by(name: 'invite') Templates.find_by(name: 'invite').call(user: user) end
- fknorangesite 3y ago
  Yes, but that's because the example we were given in this thread is just a toy strawman in that each of the methods are just simple one-liners anyway. So yeah, given this literal code sample, I agree the private methods are extraneous. The point of this pattern is to extract each operation into its own described-by-name method because they're each longer and more complicated than just a `<<` operator or whatever.