Code Review #1
Code Review #1
Conversation
There was a problem hiding this comment.
All in all, this is a great project, Chris. My comments are mostly around library functions you should know about, some stylistic things, throwing type-safety at the problem, and how to avoid partiality. Based on what I've seen here, I'd happily hire you for any team I was working on!
|
|
||
|
|
||
| -- | An angle as measured in degrees (rather than radians) | ||
| type Degree = Number |
There was a problem hiding this comment.
Type synonyms (via the type keyword) provide no type safety, and client code doing math is the exact sort of place you'd expect people to accidentally use rads instead of degrees or vice versa.
Using a newtype here would differentiate Degree from Number at the type level (rather than just changing the word used in the documentation.) I like to think of this analogously defensive driving --- assume someone (possibly you?) is going to make a mistake down the line, and you want to be ready for it.
| CenterSouth -> anchorCenterSouth s | ||
| Bottom -> anchorBottom s | ||
| LogicalOrigin -> s{pos=s.pos :-: s.originOffset} | ||
| _ -> s |
There was a problem hiding this comment.
No wildcards! Enumerate all the cases!
| updateSprites :: (Sprite -> Sprite) -> ApplicationState -> ApplicationState | ||
| updateSprites f state = state{sprites=map f state.sprites} | ||
|
|
||
| updateWhen :: (Sprite -> Boolean) -> (Sprite -> Sprite) -> SpriteMap -> SpriteMap |
There was a problem hiding this comment.
More composable to write this function as: updateWhen :: (Sprite -> Bool) -> (Sprite -> Sprite) -> Sprite -> Sprite. This thing admits a monoid instance via Endo, and now you can change predicates and transformations.
|
Closing this (but not deleting!) because it keeps popping up in my list of open PRs and I am momentarily confused about why! Hope it was helpful :) |
|
Hey! Yeah, seriously, super helpful! A very ill-timed death of my (admittedly ancient) computer right after I emailed you took a bit of steam away from my working on this (thus the sudden stop in movement). However, I've legit read your comments multiple times. Tons of useful info that I'm excited to dig into now that I've got my dev machine stood back up! Thanks again! |
|
My offer still stands if you have more questions --- I legitimately get a blast out of this stuff. Hope the new dev work goes swimmingly! |
I just deleted everything so that I could revert it and use the PR code review tools. GH is not so helpful at reviewing code that already exists :)