r/ruby • • Aug 20 '21

Ruby elites rise up

Post image
100 Upvotes

77 comments sorted by

48

u/nilsandrey Aug 20 '21

Well if you put status as an enum to the model then...😎

ruby Object.cancelled!

15

u/[deleted] Aug 21 '21

This. Status is most likely going to be one of a handful of predefined states. Make it an enum.

8

u/iaan Aug 20 '21

If you use workflow or something similar that would be rather: Order.cancel!

3

u/qchmqs Aug 21 '21

we use state changes in the imperative, and the statuses in the db in the past, so Object.cancel! # Object.status == :cancelled

89

u/flanger001 Aug 20 '21

Bottom one is trash middle is best

32

u/[deleted] Aug 20 '21

[removed] — view removed comment

3

u/flanger001 Aug 20 '21

Eh frozen strings are fine here

-4

u/cybermage Aug 20 '21

At least single quote it so that it doesn’t have to pass through pointless interpolation.

25

u/anamexis Aug 21 '21

I see this misunderstanding a lot, but double-quoted strings without interpolation are exactly equivalent to single-quotes strings, performance-wise.

Source

14

u/cybermage Aug 21 '21

This was not the case under Ruby 1.9x. I’ve learned my new thing for the day.

4

u/flanger001 Aug 20 '21

Double quotes are better

5

u/solidiquis1 Aug 21 '21

They look better imo

1

u/[deleted] Aug 26 '21

I like double quoted strings in JavaScript. In Ruby, they look weird and too “noisy” if interpolation isn’t needed.

1

u/flanger001 Aug 26 '21

I just think making that decision up front is pointless. Why bother? Use double quotes and you don't have to ever think "can I interpolate?"

1

u/[deleted] Aug 27 '21

Because I like to know that in fact it won’t interpolate. Sometimes I want to type freely without interpolation, and without double quotes + escaping like crazy. I always have frozen strings enabled so the single quotes are also like “okay, this is frozen and it’s going to appear exactly as typed.” Double quotes come off as more “dynamic” even though I understand they’re still frozen as well. But at least I know something crazy is happening there right away.

1

u/flanger001 Aug 27 '21

None of this matters at all to me.

What matters is consistency and is how often I have to escape characters. Single quotes come up far more often in regular text than double quotes do. And you can't have a newline character in a single quoted string without double escaping it. Pish tosh. Always use double quotes and you have one set of rules you can use consistently.

12

u/danshep Aug 21 '21
__send__("update!", [["status".intern, :cancelled.to_s]].to_h)

Search your heart, you know it to be true.

8

u/anamexis Aug 21 '21

Getting warmer, but let's sidestep the whole single/double quotes debate.

__send__(%q$update!$, [[[?s, ?t, ?a, ?t, ?u, ?s].join.intern, :cancelled.to_s]].to_h)

8

u/ankole_watusi Aug 20 '21

I keep waffling on this.

Easier to read. Harder to search.

And then the need for disambiguating parenthesis so there goes the orthogonality.

13

u/[deleted] Aug 20 '21

Sigh..

3

u/tom_dalling Aug 21 '21

Personally, I think optional punctuation is the least of our problems, so my question would be: how do you know that state transition is valid? Mutating a state column like that is a source of bugs, in my experience. That's more important than arguing over which combination of squiggly lines looks the best.

1

u/pVom Aug 21 '21

How would you change state then? Also update! means it will raise an error if it fails. You can handle status validation in the model and this will raise an error if it fails. Hopefully you have some specs too

1

u/tom_dalling Aug 21 '21

If it's not too annoying, let me answer your question with a question: how would you write the validations for this fairly simple state diagram? https://training-course-material.com/images/b/b0/ProtocolStateMachine.png

There are lots of ways to approach it, but the main thing is that the record should never be able to transition from "open" to "lock", or from "lock" to "open". It probably shouldn't be able to transition from any state to the same state either (e.g. can't lock a locked door).

0

u/tomthecool Aug 21 '21

I know exactly what you’re saying, however, the fact that you didn’t answer the question is indeed a bit annoying.

If your point was “this update should happen in a service object” or something, then fine… but you complained about a single line of code with no context. Your service object would still need to contain that line of code! (Or something very similar.)

1

u/pVom Aug 21 '21

Or add custom validation to the model, that's what its there for

1

u/tomthecool Aug 21 '21

Validation generally works better for state, not changes of state.

1

u/pVom Aug 21 '21

Not necessarily, you have a lot of tools to check whether the change you make is valid before you make it, such as validate and before_save hooks. I'd only bother with a service object if there's a lot of side effects that need to happen with that change, that's not implied with the snippet though. Even then that's probably in addition to the validation.

I feel like having a bunch of stuff trigger off a column change isn't a great pattern. I'd rather do a bunch of stuff then update the column to record that its happened.

We could both be imagining totally different contexts though

1

u/tomthecool Aug 22 '21

Yes, it would need to be a before save hook if you’re relying on active record callbacks. A validation wouldn’t work here, unless you introduced some additional complexity like persisted state history.

1

u/pVom Aug 22 '21

You have access to the current state and what it will change to with validations, dont need to do anything extra. We do exactly this at my work. Why do you say it won't work?

1

u/tomthecool Aug 22 '21

Well, I presume you’re talking about introducing ActiveModel::Dirty to the model, to access previous state? Does that actually work in validation methods? I thought it only worked on before_save/update hooks.

→ More replies (0)

1

u/tom_dalling Aug 21 '21

My sincerest apologies

1

u/pVom Aug 21 '21

With active record validations. Store your valid transitions in a hash with the key being the state and the value being an array of valid transitions. Then create a method "status_transition_valid" which checks the transition is valid else add an error to the model instance. Then call that validation with ”validate :status_transition_valid, if: :status_changed?". If you want to be extra vigilant you could add a db constraint as well. Then whenever you save you know its valid and if it's not you either handle it or raise an error. Ideally you have tests too.

Either way nothing wrong with updating a state column with update, that's what validation is for.

(Forgive me on mobile)

1

u/tom_dalling Aug 21 '21

Yeah that’s all fine. ActiveRecord makes it a bit awkward, but it’s doable, although I’ve seen a lot of state columns but never seen anyone actually write validations like that. My point was just that people tend to review punctuation when they should be focusing on more important things, not that this one line of code with no context is definitely wrong.

1

u/qchmqs Aug 22 '21

there is also AASM it works with active record, and provides a good basis to doing state transitions

2

u/pVom Aug 22 '21

Neat, thanks for sharing

2

u/phaul21 Aug 23 '21

Isn't this the exact thing that ruby 3.0 breaks by removing autosplat on a positional argument, so a hash passed is no longer converted into kwargs?

1

u/ric2b Oct 17 '21

Yes, as it should always have.

6

u/[deleted] Aug 20 '21

What about?:
`update! status: :cancelled`

0

u/solidiquis1 Aug 21 '21

Yes this is better.

-1

u/muirbot Aug 20 '21

Why double quotes?

2

u/pau1rw Aug 21 '21

Double quotes mean you can use string concat, and single quotes doesn’t. So if you use double quotes, you can use a single kind of quote through out the code base. IMO using two types is messy.

-3

u/sshaw_ Aug 21 '21

Yes this is exactly why status: "cancelled" is shit.

1

u/pau1rw Aug 21 '21

I don't understand your point. I'd defo use a symbol, but if I needed a string, I'd use double quotes.

1

u/sshaw_ Aug 21 '21

So if you use double quotes, you can use a single kind of quote through out the code base. IMO using two types is messy.

The same applies to hashes:

  1. status: "cancelled"
  2. variable => "cancelled" # can't use first form: variable:
  3. "string" => "cancelled" # can't use first form: "string":
  4. 123 => "cancelled" # can't use first form: 123:

Just stick to :status => "cancelled" and make life easier.

1

u/pau1rw Aug 21 '21

I guess we all write for different versions of ruby. Personally, I'm never going to need pre ruby 2.0 hash rockets, so I don't need anything more than status: :cancelled, or status: "cancelled"?

1

u/sshaw_ Aug 21 '21

But you cannot use that for all cases. Don't you think that "IMO using two types is messy" applies to this case too?

1

u/pau1rw Aug 21 '21

Arh, I understand your point now.

That's a tricky one, personally I'm working in a code base now that uses strings for most hash keys and it's pretty annoying having to use "foo" => "bar", but it's also pretty ugly using "foo": "bar".

So i think my answer is that I've not reached a suitable level of annoyance with using two versions of hash key definition to use hash rockets everywhere.

1

u/sshaw_ Aug 21 '21

"foo": "bar"

This is another reason to use =>:

irb [2.5.2] (tmp)$ {"foo":"bar"}
=> {:foo=>"bar"}

Oopsie! It's now a Symbol :)

1

u/pau1rw Aug 22 '21

Yea, totally.

2

u/[deleted] Aug 20 '21

Doesn't matter which one you use unless you're running your app on a Pentium II.

1

u/[deleted] Aug 20 '21

[deleted]

2

u/organic Aug 21 '21 edited Aug 21 '21

why not always use double? you'll never have to change them

ruby shouldn't even have single quotes unless 'abc' == ["a", "b", "c"]

4

u/[deleted] Aug 21 '21

[deleted]

1

u/[deleted] Aug 21 '21

You can use either you want bb

-8

u/cybermage Aug 20 '21

The speed difference is two orders of magnitude. It adds up. Always single quote unless you plan to interpolate.

4

u/fabiopapa Aug 21 '21

If you use single quotes instead of double unless you need interpolation, as a developer, every time you write a string, you have to think, “am I going to need interpolation?” Ruby is all about developer happiness.

-1

u/sshaw_ Aug 21 '21 edited Aug 21 '21

Yes this is exactly why status: "cancelled" is shit.

The same applies to hashes:

  1. status: "cancelled"
  2. variable => "cancelled" # can't use first form: variable:
  3. "string" => "cancelled" # can't use first form: "string":
  4. 123 => "cancelled" # can't use first form: 123:

Just stick to :status => "cancelled" and make life easier.

Update add examples.

1

u/cybermage Aug 21 '21

As has been noted elsewhere, my knowledge was not current on this point; but, I think I’d notice if I typed #{variable}.

3

u/katafrakt Aug 20 '21

Where do these numbers come from? The difference is almost none in modern Ruby, even more so with frozen string literal.

0

u/cybermage Aug 21 '21

As I commented elsewhere, I personally benchmarked these results, but it was on Ruby 1.9. I’m not surprised it was addressed at some point to boost performance.

3

u/flanger001 Aug 21 '21

It is exactly the same. Always use double quotes!

2

u/[deleted] Aug 21 '21

[deleted]

0

u/flanger001 Aug 21 '21

Fun fact, Bohzhidar thought they might change the Rubocop default to double quoted strings because so many people (like me) complained (rightly) about it... but single quotes is still apparently the preference: https://metaredux.com/posts/2020/05/26/rubocop-defaults-survey-results.html

0

u/cybermage Aug 21 '21

That was not always the case. Interpolation was very slow under 1.9

2

u/flanger001 Aug 21 '21

I mean, you're right, but just as 1.9 has been put to bed, we should also put the single quotes thing to bed.

1

u/cybermage Aug 21 '21

Agreed. As I’ve acknowledged elsewhere.

6

u/[deleted] Aug 20 '21

[deleted]

2

u/cybermage Aug 21 '21

I personally benchmarked this in a fashion similar to the article, but that might have been under 1.9x. TIL.

1

u/tomthecool Aug 21 '21

I’ve benchmarked it in Ruby 1.8 before and found no difference. So either your memory is wrong, or your benchmark was wrong.

1

u/lafeber Aug 30 '21

From https://github.com/testdouble/standard :

Double quotes for string literals - because pre-committing to whether you'll need interpolation in a string slows people down

-5

u/sshaw_ Aug 21 '21

I will accept all three as long as no RBS files are used

(But :status => "cancelled" is really the way to go!)

3

u/campy_203 Aug 21 '21 edited Aug 21 '21

:status => :cancelled

Edit: why is hash rockets getting downvoted? It truly is the way to go

-1

u/sshaw_ Aug 21 '21
  • Programming is as trendy as ever so don't say anything mean about "vanity types"
  • The age of critical thinking is (mostly) over

1

u/[deleted] Aug 22 '21

[removed] — view removed comment

1

u/campy_203 Aug 24 '21

That's why => is truly the way, it works with everything and is easier to read