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

Dynamic Functions #17

Closed
elbrujohalcon opened this issue Jun 26, 2014 · 3 comments
Closed

Dynamic Functions #17

elbrujohalcon opened this issue Jun 26, 2014 · 3 comments

Comments

@elbrujohalcon
Copy link
Member

Rule

Don't use dynamic function calling outside of behaviour modules with callbacks

Options
  • Ignore modules
@elbrujohalcon elbrujohalcon added this to the 1.0.0 - With all da rulez! milestone Jun 26, 2014
elbrujohalcon added a commit that referenced this issue Jul 21, 2014
[Closes #17] Implemented invalid dynamic calls rule.
elbrujohalcon added a commit that referenced this issue Jul 21, 2014
[Closes #17] Implemented 'used ignored variable' rule.
@mpmiszczyk
Copy link

On one hand great rule. We found with it few places we could improve our code with it.

On other we have some issues with this one. Especially with making it go away. This one assumes that all callback have to be defined in module that uses them. And this is not always our case.

Personally I prefer to create separate module that just have callback. Like this one. This is especially useful when more than one module uses same kind of behaviour.

Second use case is when one module uses two types of behaviors. For example we could have generic algorithm that handles interactions of cats and dogs. So you could have both calls like CatModule:boots() and DogModule:with_bone(). So you should have two different behaviors defined, but one module can define only one.

Our solution (not to pass Elvis rule, but rather to keep code clean/readable) would be specs. To be exact we define and export type from callback module, like type cat_module() :: module()., which is enough to keep functions readability (without any actual type-checking of course).

So maybe some modification of this rule would be a good idea ?

@mpmiszczyk
Copy link

There is a third option:

We define callback module, with functions calling callbacks:
https://github.com/ParaPhraseAGH/parallant/blob/simplify/src/world_impl.erl

And all the clients call callback-module which calls callbacks implementations.
https://github.com/ParaPhraseAGH/parallant/blob/simplify/src/parallant.erl#L135

Not sure if you had in mind such pattern when you designed this rule, but this is good for both to Elvis rule, and our use case problems. And i think I like it, even little more than dynamic calls.

Huge kudos to @pianiel for coming up with such solution.

@elbrujohalcon
Copy link
Member Author

@mpmiszczyk I understand your two scenarios and, to cope with things like that is why we added a list of module exceptions to the rule. Because we couldn't find a different way to be sure that a call to Module:some_function() will be valid because Module will be implementing a behaviour that defines some_function/0.

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

No branches or pull requests

2 participants