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

Support for modules included in a class #3

Open
paulcsmith opened this issue Mar 19, 2019 · 4 comments
Open

Support for modules included in a class #3

paulcsmith opened this issue Mar 19, 2019 · 4 comments

Comments

@paulcsmith
Copy link

Let's say I have a module that is included in a class:

module Paramable
  include Cannon::Auto

  abstract def parse
end

class JsonParams
  include Paramable
end

class FormEncodedParams
  include Paramable
end

And I make an instance var of the module type

class Action
  include Cannon::Auto

  @params : Paramable
end

The problem is that Paramable is a module so you can't define instance methods on it that Cannon expects.

Is there a way to do this? And if not, do you have an idea of where I could start? I'd love to try a PR

@Papierkorb
Copy link
Owner

Hello,

Been a long time since I took a look at the code. But interesting use-case you have there!

The Cannon::Auto module adds a #to_cannon_io method, which in the
default inception calls Cannon.encode in-order on all instance variables.
That Cannon.encode then tries to encode it, in case of your Paramable
it should fall into the case to call #to_cannon_io on your value. This should
be fine, however, doing the reverse probably breaks. Haven't tried this, but
is this about what's going on? Or what error message do you get?

In any case, what you'd have to do then is to modify the Cannon.encode
and Cannon.decode methods to check if the T it gets is a module type,
and if so, should construct an Union(...) of all implementing types. This
Union(...) mapping is much like the first case in those functions, in fact,
it may be enough to add this processing and then just use the existing
Union encoder to get the job done.

Cheers!

@paulcsmith
Copy link
Author

Hi there!

Thanks for the quick response. Here is the exact error. It looks like it is already using a Union, but maybe it is defining the wrong method?

instantiating 'Cannon:Module#encode(IO::Memory, Avram::Paramable)'
in lib/cannon/src/cannon/serialisation.cr:18: wrong number of arguments for 'Avram::Paramable.to_cannon_io' (given 2, expected 1)
  - Cannon::AutoToIo#to_cannon_io(io)

   Union(typeof(value)).to_cannon_io(io, value)

@Papierkorb
Copy link
Owner

It looks like it is already using a Union

Would say so as well, but I guess that typeof(value) returns the Paramable module making that a Union(Paramable), which isn't much useful. Even if this call would go through, the issue is that upon
decoding the program would have no idea what kind of Paramable is now following and thus would
go into undefined behaviour territory (More than cannon intends ;-) ).

You'd have to modify the Cannon.encode and Cannon.decode methods to account for this and
generate code, using macro magic, that would treat your module MyMod to be a Union(AllStructsAndClassesThatIncludeMyMod). Then it could go through the existing Union code-path which does this type mapping at run-time.

@paulcsmith
Copy link
Author

Thank you for the help. I'm not sure I'll be able to do that, but I'll give it a shot :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants