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

Testers, filters, and global functions should be traits #219

Open
SergioBenitez opened this issue Oct 10, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@SergioBenitez
Copy link
Contributor

commented Oct 10, 2017

At present, all of these have signatures that require either static function (testers, filters), or static function/closures (global functions). Instead, all of these should use traits so that any structure with arbitrary state can be used as a test/filter/global function handler. Here's what I'm imagining:

trait Tester: 'static {
    fn(&self, _: Option<Value>, _: Vec<Value>) -> Result<bool>;
}

trait Filter: 'static {
    fn(&self, _: Value, _: HashMap<String, Value>) -> Result<Value>;
}


trait GlobalFn: Send + Sync + 'static {
    fn(&self, _: HashMap<String, Value>) -> Result<Value>;
}

For backwards compatibility and ease-of-use, you can provide a blanket impl for fn-like types. For instance, for Tester, we can do:

impl<F: 'static> Tester for F where F: Fn(Option<Value>, Vec<Value>) -> Result<bool> {
    fn(&self, a: Option<Value>, v: Vec<Value>) -> Result<bool> {
        self(a, v)
    }
}
@Keats

This comment has been minimized.

Copy link
Owner

commented Oct 15, 2017

Did it come up as something asked for Rocket? That seems ok to me.

@Keats

This comment has been minimized.

Copy link
Owner

commented Oct 25, 2017

@SergioBenitez Do you have a concrete example for the changelog from actual use? I'll probably implement it this week

@remexre

This comment has been minimized.

Copy link

commented Aug 25, 2018

What'd the status on this end up being? Did there turn out to be some hidden pitfall to using traits?

@Keats

This comment has been minimized.

Copy link
Owner

commented Aug 25, 2018

I don't think there are any pitfalls, just didn't have the time/forgot about it. I would take a PR for that though.

@Keats Keats referenced this issue Sep 6, 2018

Open

v1 release #331

11 of 16 tasks complete

@Keats Keats added the done label Jan 4, 2019

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