Poor Man's Type Checking in Arguments and Returns

[julitrows]

Please, disregard the branch name 😮‍💨

Changes

Add optional type checking to argument declarations

argument :jarl, type: Chiquito
argument :torpedo, type: Array, default: nil

Add optional type checking to returned values

returns MyClass, nullable: true
returns MyCollection, of: Item, nullable: true, allow_nils: false

Notes

Made them optional, since they're not the main purpose of the library, but happy to cut a 3.0.0 branch with breaking changes.

See each commit's README entries for more examples

class UserByPermissionsQuery
  include Injectable
  
  argument(:permission, type: Permission)
  dependency(:base_relation) { User.all }
  returns(ActiveRecord::Associations::CollectionProxy, of: User, nullable: false, allow_nils: false)

  def call
    base_relation.includes(:permissions).where(permissions: { id: permission.id })
  end
end

About Collection validations

I've got a bit of a mess there. Not really sure I should check for Enumerable, or just responds_to?(:each) is enough?

Maybe having an optional enumerates_with: key is cool to indicate which method is used to iterate the collection (default :each)?

returns MyCollection, of: Item, enumerates_with: :each, nullable: false, allow_nils: false

[iovis]

Not a review, but to answer some of your questions:

Made them optional, since they're not the main purpose of the library, but happy to cut a 3.0.0 branch with breaking changes.

My idea originally was for it to be opt-in, since it has a runtime performance hit (not sure how much, but it's doing more work than before), and not everyone wants/likes strong typing.

I've got a bit of a mess there. Not really sure I should check for Enumerable, or just responds_to?(:each) is enough?

Maybe having an optional enumerates_with: key is cool to indicate which method is used to iterate the collection (default :each)?

I thought you'd do a collection.all? { it.is_a? type }. In Enumerable those are still defined in terms of :each, so I think it's a fair assumption. Still, I would assume that if someones uses of: that they're passing a collection and I would assume :each. I'd start simple and add complexity as you need it, so I'd personally keep : enumerates_with in the back burner.

[julitrows]

@iovis WDYT?

[aalbagarcia]

Hey Julio!!

It took me some time but I finally did it.

To answer your questions. I prefer checking for :each. It's more ruby-ish than chekcing for Enumerable. So I'd keep it as you did in the lastest changes.

I'd keep the type checking optional. It would be a bit unnatural for me to use a library in ruby that forces me to use types. Also agree with iovis comment about the impact on performance, it would be something to double check if types become a non-optional thing.

Finally, I'm thinking of a use case that might be needed: what If I need the argument to be of different types?

class MyClass
  include Injectable

  argument :user, type: [User, AdminUser]
end

I guess the answer would be to add two arguments and a validation to check that at least one exist:

class MyClass
  include Injectable

  argument :user, type: User, default: nil
  argument :admin_user, type: AdminUser, default nil

  validate :user_exists

end

Maybe something to add to the readme if you don't want to implement this use case.

It looks good to me. Left a couple of comments about naming, but they are not blocking.

[iovis]

@iovis WDYT?

I completely forgot about this till I got the notification from @aalbagarcia's comment, lol.
Sorry about that!

I haven't checked the new changes yet, but I remember seeing this project over the holidays: https://github.com/low-rb/low_type

I haven't checked what kind of black magic they're doing behind the scenes yet, but the syntax they use for defining types was much closer to the "ideal DSL" I had in mind when I thought of this feature.

I wonder if we can borrow some ideas from their implementation!

[julitrows]

Hey @aalbagarcia and @iovis thanks for the comments.

I will:

1. Enhance further typing of arguments so an argument can be a collection with elements of a kind, like we do for returns. @aalbagarcia 's example made me think of it.
2. Enhance further typing of arguments so an argument needs to be be one of the classes provided in an array
3. Apply comments on README, names of keys and how validations are called, as per @aalbagarcia's comments ✅
4. Provide benchmarks to see how this behaves.
5. Look into low_type and see how I can do something similar, or even, leverage the gem itself?

Cheers!