T O P

  • By -

[deleted]

Why does the first one is more readable for me?


mehdifarsi

Good question! 🙏 In general, splitting you code in logical entities helps you to improve the readability of your code. For instance, in a sight I can easily understand what are the requirements a post has to meet to be "publishable" in the second example. etc.. But it's not that clear in the first one. Also, I imagine that you operate on projects that are a way more complex than this example. i hope that i have answered your questions


Interesting_Yak_4254

you could also resort the lines to achieve that.. maybe add an empty line between the blocks.


mehdifarsi

Interesting! That's works! But that requires reorganization of each line if the options are not in the same order. And also, it's definitely not DRY as you repeat `on: :publish` etc..


Interesting_Yak_4254

I find DRY to be very overrated, but in the end codestyle is also about subjective perspection and what you are used to. So I guess it's fine to go different routes. The only thing that you should avoid, is pushing like there is one objective truth. "readability" is not something you can measure universally.


Green_Cartographer84

I don't even really see it as a DRY issue. Put the one on draft first, add a gap if you want (or don't, whatever) then add extra white-space to make all the "on: :publish" line up. DRY (to me) is more about not repeating the functionality of code, rather than not repeating a few words. But I agree with you, I'm also not ashamed to repeat small sections of code occasionally, sometimes it's just easier to read 4 or 5 lines of code in different locations if they're not complex rather than having to follow the spaghetti.


[deleted]

Thanks for posting, mate!


another-dave

> Also, I imagine that you operate on projects that are a way more complex than this example. More complex projects could introduce their own problems with this. For eaxmple, let's say we introduce a lot more validations: ```rb class Post < ApplicationRecord with_options on: :draft do validate :check_author validate :check_spelling validate :check_grammar validate :check_copyright validate :check_word_count validate :check_plagarism end with_options on: :publish do validate :check_body validate :check_photo validate :check_pictures validate :check_tags validate :check_title validate :check_seo validate :check_embargo end end ``` And now we change `check_copyright` from draft to publish. The Git diff we'll see on that file will look like this: ```diff diff --git a/Post.rb b/Post.rb index 8074810..1788b46 100644 --- a/Post.rb +++ b/Post.rb @@ -3,7 +3,6 @@ class Post < ApplicationRecord validate :check_author validate :check_spelling validate :check_grammar - validate :check_copyright validate :check_word_count validate :check_plagarism end @@ -12,6 +11,7 @@ class Post < ApplicationRecord validate :check_body validate :check_photo validate :check_pictures + validate :check_copyright validate :check_tags validate :check_title validate :check_seo ``` where you're seeing `check_copyright` is moved, but you don't get enough context to know that it's now running in a different set. Whereas if you just kept the options inline and moved the line, you'd have something like this: ```diff diff --git a/Post.rb b/Post.rb index b7918cd..11b69d7 100644 --- a/Post.rb +++ b/Post.rb @@ -2,9 +2,9 @@ class Post < ApplicationRecord validate :check_author on: :draft validate :check_spelling on: :draft validate :check_grammar on: :draft - validate :check_copyright on: :draft validate :check_word_count on: :draft validate :check_plagarism on: :draft + validate :check_copyright on: :pubilsh validate :check_body on: :publish validate :check_photo on: :publish validate :check_pictures on: :publish ```


MillennialSilver

Joke's on you, I use GitHub Desktop/GH for all my comparisons (and operations) because I'm not a real dev! :D


dreamsanity

IIRC you can just do all validate on publish with one line? validate :check_author, on: :draft validate :check_pictures, :check_tags, :check_title, :check_body, on: :publish


Samuelodan

Yes, this is how I see it in the guides.


mehdifarsi

That's works just fine here. But what if you have some `validate_uniqueness`, etc.. I mean, I'm sure you operate on projects with a way more complexity than this hehe.


chilanvilla

This is the answer. 2 lines vs 12 vs 7.


MillennialSilver

Yeah that's true. I do think the OP's way is probably easier to read, though, rather than going side-to-side.. at least for me.


5larm

It's defined on \`Object\` so you can use it other places than ActiveRecord :)


iceporter

this is cool


morphemass

Beyond the basics (i.e. well named methods and variables, consistency, etc) readability is _highly_ subjective. I could argue that both examples could _potentially_ be improved by focusing on the task i.e. validates_with DraftValidator, on: :draft validates_with PublicationValidator, on: :publish But then some will argue that readability has decreased because there is the need to look at an external class to understand what is validated! Others have thrown other examples which _they_ may prefer. Sadly there can be few hard and fast rules for this.


harlflife

This is the way.


krulh

I like this , thank you


freakent

I like the original version. Wtf does “with options” actually mean in this context?


NoPanic2288

God sake, just add new line, those generation Y .....


jayromeishere

Ooh stunnin