I had passed the phone screen for a chance to work as a ClojureScript developer for a startup in manhattan, and they emailed me a coding challenge. Basically, they wanted me to make a glorified do-to list using reagent and re-frame. This post is to document the feedback from the reviewer of my code and think out loud about what he / she meant by it and what I can do to avoid the same issues the future.
Feedback Letter
I wanted to pass along some constructive feedback on your Derpframe code exercise. Here are a few (hopefully helpful bits of feedback):
Hope that is helpful to you.
- We felt the code wasn’t very DRY, probably because you could have used some more abstractions
- You left in namespaces that were blank or commented out
- You had the same ID for everything in the product list. We found this confusing.
- Lots of (nth item 1) and nested (get)s made things a bit hard to follow.
- Various formatting issues made this feel rushed/unfinished.
Hope that is helpful to you.
Reflections
I'm still not totally sure what he meant by (1). I did break break the view elements out into presentational and container components, but possibly I could have extracted things into smaller functions? They must have thought there were places I was repeating myself. I'm sure (2) could have easily been avoided with an editor that somehow highlights unused namespaces being required. Also (5), the general formatting issues, could have been avoided if I was able to just Ctrl + Alt + L as I do in JavaScript. I really like to tear apart the code in order to modify it, and having to manually move parens around gets tedious very quickly. Not sure why they were seeing the same id for every product (3) since this exact thing was initially giving me an error in the browser console, but I then changed code to force each one to have a unique id. (4) talks about using (nth item 1), and I know exactly where in the code that occurs. It's really just some lost residue lingering from when I had originally designed the app to allow the user to add any quantity of the item to the cart, but then near the end of development I realized the original spec called for only needing to add 1 item to the cart. Of course looking back I can see that if the index is always 1 then it's probably clearer and easier to read if you just say (first item). The nested gets were also changed to get-in's as part of the refactorings in a later commit.
Key Takeaways
If I want to be serious about professionally developing Reagent / Re-Frame applications I should really have a good editor with solid code reformatting tools and use a Clojure linter. Never ever type (nth item 1) with a hardcoded 1 again! Also, try to use get-in instead of nested gets right from the beginning instead of having to refactor them in a commit after the one being looked at by the person judging your code. ;) Stay strong, bend over, and take all the criticism you can get because these are the things that will make you wiser. You'll encounter plenty of "No"s on the the road to greatness. Just try to learn as much as you can along the way.
If you're curious, the code for my project is hosted here.
If you're curious, the code for my project is hosted here.
RSS Feed