WatchApps icon indicating copy to clipboard operation
WatchApps copied to clipboard

Heart app correlator typo

Open ddelemeny opened this issue 2 years ago • 2 comments

Hi!

https://github.com/jeffmer/WatchApps/blob/d8dff30062ebdb14a727d5340129a68471f069eb/apps/heart/heartv2.app.js#L34

I think this line should be written int a = (next-c) % NSLOT; as the current form will be out of bounds when next == c

ddelemeny avatar Aug 29 '23 13:08 ddelemeny

Thanks, I think you are right about the out of bounds, however, your code would give negative indices when c is greater than next as -4%8 is -4 for example.

I guess it should be: int a = (next-c)>=0? (next-c) : (NSLOT+next-c);

Fixing it should make improve the accuracy of the correlation as the out of bounds location will have a random value.

jeffmer avatar Aug 31 '23 13:08 jeffmer

Oh, that's a nice language specific quirk I didn't know about: the modulo operator may or may not return a signed value depending on the language.

In JS and C the operator is implemented using the truncated definition, therefore -4%8 returns -4. In python (which I'm more familiar with and tend to use for quick and dirty checks) it uses the floored definition and returns 4.

TIL.

https://en.wikipedia.org/wiki/Modulo

ddelemeny avatar Sep 03 '23 11:09 ddelemeny