lume icon indicating copy to clipboard operation
lume copied to clipboard

logic functions

Open grunkgrunk opened this issue 4 years ago • 3 comments

Hey! Thanks for creating this library, it's awesome!

I have added some "functional" versions of the logical operations and, or and not. These can for example be used together with lume.filter. To get all enemies with low health that have just been damaged we might call

lume.filter(actors, lume.And(hasLowHealth, isEnemy,  wasJustDamaged)  )

I realize the capital letter in And is a bit at odds with the naming convention of the library. However I was not allowed to name them in all lowercase letters (which makes sense I guess). If you have any suggestions about naming or anything else I'm happy to change things.

If these functions end up being part of the library I'll of course write some documentation as well. If not then that's of course totally fair :))

Cheers!

grunkgrunk avatar Aug 08 '21 09:08 grunkgrunk

Maybe just my personal taste, but if this is purely for aesthetics I find using lume.all and lume.any to be easier to read:

-- All of these
lume.all({ hasLowHealth, isEnemy,  wasJustDamaged })

-- Any of these
lume.any({ hasLowHealth, isEnemy,  wasJustDamaged })

jaythomas avatar Aug 09 '21 18:08 jaythomas

Right, but hasLowHealth and the others are functions. Sorry I forgot to mention that. So I don't think your approach would do the same thing? What lume.And does is that it creates a new function - in this case it creates the function f(x) = hasLowHealth(x) and isEnemy(x) and wasJustDamaged(x).

grunkgrunk avatar Aug 09 '21 20:08 grunkgrunk

No sorry that's my fault as I didn't read the diff all the way through before commenting. Your approach makes perfect sense then for your scenario since you're providing those predicate functions to lume.all/lume.any to build these higher order functions.

On a deeper level, I don't know what @rxi's criteria is for accepting new functions to this library. Being this library and your change is tiny there is little worry about bloat, but I can also see the argument in perhaps splitting lume's monolith up into separate rocks ala lodash where you can either require the entire library (require('lodash')) or cherry-pick just the functions you want (require('lodash.reduce')) to cut down on application size and memory footprint. That would allow features like yours to be accepted into the fold with less worry about bloat.

jaythomas avatar Aug 10 '21 00:08 jaythomas