timshannon / bolthold

BoltHold is an embeddable NoSQL store for Go types built on BoltDB
MIT License
648 stars 46 forks source link

consider an alternative query.Where("").Match() #49

Closed mh-cbon closed 5 years ago

mh-cbon commented 5 years ago

Hi,

instead of MatchFunc(func(ra *RecordAccess) (bool, error)) can you consider an alternative Match(func(<T>) bool) ?

Where <T> is user defined and you just have to check the type compatibility before invoking the given function via reflect.

That would be a really cool improvement over the api. This matcfunc implementation has smelly taste IMVHO.

I sketched an example impl here https://play.golang.org/p/t-VvzLu6wC8

timshannon commented 5 years ago

I agree for the most part however:

I think it's worth doing. What I'm tentatively picturing is to change the MatchFunc signature to:

MatchFunc(func(value interface{}) (bool, error)

If value is of type RecordAccess, work just like it does now. If value has type compatibility with the entire record, return the record. If it has type compatibility with the currently selected field, return the field.

So you could do something like this:

bolthold.Where("Name").MatchFunc(func(val string) (bool, error) {
            return val != "one" && val != "two" && val != "three"
        })

or

bolthold.Where("Name").MatchFunc(func(val person) (bool, error) {
            return val.Name != "one" && val.Name != "two" && val.Name != "three"
        })
mh-cbon commented 5 years ago

maybe you dont break api and you add a new handler ?

timshannon commented 5 years ago

The problem with adding a new handler is that it makes the API more complicated, especially for people coming to it for the first time. What would you name the new handler that made it clear that this new MatchFunc handler also matches on the passed in function, but in a slightly different way than the other handler that matches on a function.

I'd prefer to just extend the behavior of the existing handler without changing the current behavior.

timshannon commented 5 years ago

The changes discussed above have been moved into master.