Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add type names as class methods (`Types.string`, `Types.integer`, etc.) #361

Closed
patrickclery opened this issue Oct 21, 2019 · 7 comments
Closed
Labels

Comments

@patrickclery
Copy link
Contributor

@patrickclery patrickclery commented Oct 21, 2019

User Story

As a developer I'd like to be able to access

  • call type objects with class methods
  • call class methods even when Dry::Types is aliased to Types

So the code is more readable, & less typing for the developer.

Examples

module Types
 include Dry.Types()
end
# old
drinking_age = Types::Integer.constrained(gt: 21)
# new
drinking_age = Dry::Types.integer.constrained(gt: 21)
# old
email = Dry::Types::String.constrained(
 format: /\A[\w+\-.]+@[a-z\d\-]+(\.[a-z]+)*\.[a-z]+\z/i
)
email = Dry::Types.string.constrained(
 format: /\A[\w+\-.]+@[a-z\d\-]+(\.[a-z]+)*\.[a-z]+\z/i
)

Not a huge difference, right? But once you've alias Types, your code changes:

# from this
drinking_age = Dry::Types::Integer.constrained(gt: 21)
# to this
drinking_age = Types.integer.constrained(gt: 21)
# or if you need more specificity
drinking_age = Types.send('coerible.integer').constrained(gt: 21)

☝️ 6 characters less, and more readable (in my opinion).

Here's a working of how I updated my tinder_client gem from using Dry::Types['string'] to Types.string:

   class Updates < Dry::Struct
 
-    attribute :blocks, Dry::Types['array'].of(Dry::Types['string'])
-    attribute :deleted_lists, Dry::Types['array']
-    attribute :goingout, Dry::Types['array']
-    attribute :harassing_messages, Dry::Types['array']
-    attribute :inbox, Dry::Types['array'] do
-      attribute :_id, Dry::Types['string']
-      attribute :match_id, Dry::Types['string']
-      attribute :sent_date, Dry::Types['string']
-      attribute :message, Dry::Types['string']
-      attribute :to, Dry::Types['string']
-      attribute :from, Dry::Types['string']
-      attribute :created_date, Dry::Types['string']
-      attribute :timestamp, Dry::Types['coercible.string']
+    attribute :blocks, Types.array.of(Types.string)
+    attribute :deleted_lists, Types.array
+    attribute :goingout, Types.array
+    attribute :harassing_messages, Types.array
+    attribute :inbox, Types.array do
+      attribute :_id, Types.string
+      attribute :match_id, Types.string
+      attribute :sent_date, Types.string
+      attribute :message, Types.string
+      attribute :to, Types.string
+      attribute :from, Types.string
+      attribute :created_date, Types.string
+      attribute :timestamp, Types.send('coercible.string')
     end
   end
 end

Resources

The way I'm able to accomplish this was by taking the existing snippet from the dry-types docs to create a Types module, then add all the type keys as methods:

spec_helper.rb

require 'dry-types'
require 'dry-struct'

# This automatically transforms keys to symbols when you pass Structs a hash
# that has string keys
class Dry::Struct
  transform_keys(&:to_sym)
end

# The following allow you to access types with Types.<type reference>
#
# ```ruby
# email = Types.string.constrained(
#   format: /\A[\w+\-.]+@[a-z\d\-]+(\.[a-z]+)*\.[a-z]+\z/i
# )
# drinking_age = Types.integer.constrained(gt: 21)
# ````
module Types
  include Dry.Types()

  class << self

    # @param String The key of the Dry::Type -- see Dry::Types.type_keys
    def [] (type_key)
      Dry::Types[type_key]
    end

    # This aliases all the Dry::Types keys as class methods
    Dry::Types.type_keys.each do |method_name|
      define_method method_name do
        Dry::Types[method_name]
      end
    end
  end

end
@flash-gordon

This comment has been minimized.

Copy link
Member

@flash-gordon flash-gordon commented Oct 21, 2019

Sorry, I don't think it's a good idea for built-in functionality. First of all, I don't see what problem this is trying to solve. Using modules with imported constants is the recommended way of using dry-types, not accessing them via container with Dry::Types.[]. Referencing types with constants is hardly a problem, those are immutable objects with clear semantics. Accessing them with methods is a matter of personal taste, I can understand this. However, I don't see how methods are better than constants for this particular case. Besides, nothing prevents you from accessing types with methods in your projects, you can even build a gem for that.

@patrickclery

This comment has been minimized.

Copy link
Contributor Author

@patrickclery patrickclery commented Oct 21, 2019

Sorry, I don't think it's a good idea for built-in functionality. First of all, I don't see what problem this is trying to solve. Using modules with imported constants is the recommended way of using dry-types, not accessing them via container with Dry::Types.[]. Referencing types with constants is hardly a problem, those are immutable objects with clear semantics. Accessing them with methods is a matter of personal taste, I can understand this. However, I don't see how methods are better than constants for this particular case. Besides, nothing prevents you from accessing types with methods in your projects, you can even build a gem for that.

@flash-gordon I would love this as a built-in feature, but how about in the docs?

Check out PR #362 it's for adding this to docs.

Having it built-in would be nice, but either way is ok for me :)

The benefit to built in is that, for me, it would be idea to install this gem without any helpers and do require 'dry-types'; int = Types.integer 🤷‍♂️

@flash-gordon

This comment has been minimized.

Copy link
Member

@flash-gordon flash-gordon commented Oct 22, 2019

Sorry, I'm still far from understanding why this might be useful. As for me, int = Types::Integer works perfectly fine.

@patrickclery

This comment has been minimized.

Copy link
Contributor Author

@patrickclery patrickclery commented Oct 22, 2019

Sorry, I'm still far from understanding why this might be useful. As for me, int = Types::Integer works perfectly fine.

My user story answered your specific question:

So the code is more readable, & less typing for the developer.

The docs are already suggesting to make things more readable by using Types::Strict::String. IMO

The main issue that could be improved is that the docs are already attempting to help make it more readable by adding the Types module. If the point is for readability, renaming Dry::Types to Types then having the same symbol-ridden syntax with colons and dots everywhere, not readable in my experience. Adding Types.string is something I wish I would've done much earlier and had the docs suggested it, I would've.

I've made my case though and it's rejected 🤷‍♂️ end of discussion then

@flash-gordon

This comment has been minimized.

Copy link
Member

@flash-gordon flash-gordon commented Oct 23, 2019

That's probably on me but I didn't even understand what you tried to do. In the docs we're trying to show how to use the library, it includes exporting constants. Please keep in mind we use GH issues only for approved feature requests, send them to discourse first. For quick hacking, we may add something else, like dry-types-console script or something similar.

@patrickclery

This comment has been minimized.

Copy link
Contributor Author

@patrickclery patrickclery commented Oct 23, 2019

That's probably on me but I didn't even understand what you tried to do. In the docs we're trying to show how to use the library, it includes exporting constants. Please keep in mind we use GH issues only for approved feature requests, send them to discourse first. For quick hacking, we may add something else, like dry-types-console script or something similar.

Gotcha thanks I wasn't really sure where to start because Zulip is often inactive. I'll move this to Discourse*, thanks :)

@patrickclery

This comment has been minimized.

Copy link
Contributor Author

@patrickclery patrickclery commented Oct 23, 2019

Moved to Discourse

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.