karamaan / karamaan-opaleye

Other
11 stars 7 forks source link

Num instance exprarr #41

Open hesselink opened 10 years ago

hesselink commented 10 years ago

I added a Num instance for ExprArr a (Wire b). This makes writing expressions to restrict queries much nicer: constants are just number literals, and all numeric operations like (+) lose the dots. I'm a bit scared about what happens when you use other, derived functions using Num. Those might introduce quite large expressions, I guess. But I think it's worth it for the numeric literals alone.

Note that this builds on top of my relax-constant-exprs branch, since that is needed for the fromIntegral to type check.

tomjaguarpaw commented 10 years ago

I think your signum actually does abs. Compare with the actual definition of abs just above it! I doubt Postgres supports a signum operator so you may just want to write it in terms of an if_.

hesselink commented 10 years ago

You're right of course. I thought it would call the sign function, but it applied the @ operator. I've changed the commit to do what I wanted in the first place.

tomjaguarpaw commented 10 years ago

I've added signum.

tomjaguarpaw commented 10 years ago

@ocharles: I remember you showed me a Num instance for the Netwire arrows in your Mario game. Did you find it worthwhile?

ocharles commented 10 years ago

Without a doubt, in FRP it's almost impossible to work without Num instances.

hesselink commented 10 years ago

@tomjaguarpaw You say you added signum, but I already added it. What exactly did you do?

tomjaguarpaw commented 10 years ago

I couldn't apply your pull request directly because it requires the unrestricted constant stuff that I don't want to add. I just added signum by hand.

hesselink commented 10 years ago

Ah, I wasn't looking on the dev branch. Looks like you refactored it a bit too, getting rid of the duplication. I'll rebase the addition of the Num instance on top of dev.

tomjaguarpaw commented 10 years ago

OK this looks totally fine, but could you try to persuade me that introducing this is not going to be a historical mistake that leads to confusing code? After all, once we've added this instance there's no going back without great pain for Opaleye users ...

(Comments from others welcome too)

hesselink commented 10 years ago

I'm not sure. I think for literals it's not problematic, Haskell users are used to overloaded numbers. For simple operations like + or * it also looks very natural, with things like arr age + 12. It's a bit to loose, in the sense that there's no Num constraint on the b but the same goes for plus now. You mentioned in an email that you wanted to introduce a type class for it, that might help.

I'll play with it a bit more, and let you know what my experiences are.

tomjaguarpaw commented 10 years ago

Thanks. It's also useful to have your branch available so others can play with it too.

tomjaguarpaw commented 10 years ago

The more I think about this the more it seems like a sensible thing to do. Have you been using it? What is your experience?

bergmark commented 10 years ago

I'm not sure if I have any code that uses this, so I can't tell.