crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.47k stars 1.62k forks source link

Allow method overloads for `enum` members #15063

Open jgaskins opened 1 month ago

jgaskins commented 1 month ago

I've found myself writing a lot of event-handling code recently. When an event comes in over a webhook (GitHub integrations, Stripe subscriptions), I often pass it off to a method so I can lean on method overloading to route the event to its handler:

abstract struct Event
  include JSON::Serializable

  use_json_discriminator "type", {
    create: Create,
    update: Update,
    delete: Delete,
  }

  struct Create < self
    # ...
  end

  struct Update < self
    # ...
  end

  struct Delete < self
    # ...
  end
end

def handle(create : Event::Create)
end

def handle(update : Event::Update)
end

def handle(delete : Event::Delete)
end

In some of these integrations, I need to do the same sort of routing on the value of an enum, and I think it would be nice if Crystal supported the same pattern there:

enum Action
  Create
  Update
  Delete
end

def handle(create : Action::Create)
end

def handle(update : Action::Update)
end

def handle(delete : Action::Delete)
end

In both cases, it saves the programmer from having to write an intermediate case … in statement to perform the routing. Depending on complexity, you're probably delegating the handler logic to another method anyway:

enum Action
  Create
  Update
  Delete
end

def handle(action : Action)
  case action
  in .create?
    handle_create
  in .update?
    handle_update
  in .delete?
    handle_delete
  end
end

def handle_create
end

def handle_update
end

def handle_delete
end

If we can use enum members as method overloads, we can lean on a construct that automatically guarantees that all cases are handled which is already used for inheritance.

straight-shoota commented 1 month ago

It might not be the most critical issue, but I'm confused by a method signature handle(create : Action::Create). Since Action::Create is an enum item, not a type, it doesn't make sense to have an argument typed as that. So I'm not convinced this proposal is a good idea in the first place. Let alone if it's worth the implementation effort.

There are other ways to avoid the case ... in boilerplate. Fore example, the following macro would do:

def handle(action : Action)
  {% begin %}
    case action
    {% for action in Action.constants %}
      in .{{ action.underscore }}?
        handle_{{action.underscore }}
    {% end %}
    end
    {{ debug }}
  {% end %}
end

def handle_create
end

def handle_update
end

def handle_delete
end
HertzDevil commented 3 weeks ago

This would be akin to literal types in languages like TypeScript, where if a value appears as a type restriction, it represents the type consisting only that value. Thus the idea can be extended to booleans, whose individual values are also supported in exhaustive case expressions:

def handle(x : true)
end

def handle(x : false)
end
jgaskins commented 3 weeks ago

This would be akin to literal types in languages like TypeScript, where if a value appears as a type restriction, it represents the type consisting only that value. Thus the idea can be extended to booleans, whose individual values are also supported in exhaustive case expressions

I think this captures what I have in mind really well, which is that if you can use it in case … in X, it would be amazing to be able to use that same X as the "type" in the method signature — putting aside for the moment that it's not a "type" as Crystal currently defines them. With exhaustive case for types, you have to account for every combination of types just like you do with method signatures. Then we can simply lean on the type system.

TypeScript and Elixir were what I had in mind when thinking about this.

Another example might be having multiple enums determining what to do. Maybe the role of the person performing the action in addition to the kind of action (very common in things like webhook event handlers to have multiple enum values determining how you handle the event):

enum Action
  Create
  Update
  Delete
end

struct User
  # ...

  enum Role
    Member
    Admin
  end
end

Currently the code would look like this, with the permissions separate from the handler methods:

def handle(action : Action, role : User::Role)
  case {action, role}
  when {Action::Create, User::Role}
    create role
  when {Action::Update, User::Role::Admin}
    update
  when {Action::Delete, User::Role::Admin}
    delete
  else
    forbidden
  end
end

def create(role : User::Role)
end

def update
end

def delete
end

def forbidden
end

What I'm proposing would let you define the permissions and the handlers together.

# Catch-all, equivalent to the `forbidden` method in the previous example
def handle(action : Action, role : User::Role)
end

# Members and admins can create stuff
def handle(action : Action::Create, role : User::Role)
end

# Only admins can update stuff
def handle(action : Action::Update, role : User::Role::Admin)
end

# Only admins can delete stuff
def handle(action : Action::Delete, role : User::Role::Admin)
end

This pattern wouldn't be ideal beyond some level of complexity, but locality can often be a feature.

ysbaddaden commented 3 weeks ago

Something akin to each enum constant inheriting from the enum itself? I mean Action::Delete < Action