Clojurians Log v2

Where was that dispatch line initially?

just above where the defnition of my game function is

Also, do you want feedback just about re-frame or also about CLJ/S or even programming in general?

yeah yeah, that too is welcome

Using dispatch at the top level of a namespace means that it will be called when the namespace is loaded - not when the desired URL is navigated to. in games.core, you have routes definitions with a home route that dispatches some init event - that will overwrite any DB state that exists at that point. A proper fix would be moving that gridalizing dispatch to the :start handler of the route. Might also make it a tad more generic and wrap the call to dispatch in some init function in relevant namespaces that are then called by the router. So you don't clump together routes and specific event IDs.

Im not sure I follow this "A proper fix would be moving that gridalizing dispatch to the :start handler of the route."

do you mean dispatch event inside (game function?

aha, like line 63 inside core.cljs

i see

yeah, that did work. thanks @p-himik. please feel invited to share more of your feedback regarding anything you see could be better.

Some other things: • Stuff like (and (<= 0 y) (> cols y)) can be written as (< -1 y cols) • I like how you organized directions inside neighbors. But it's unlikely to survive any formatting tool, so you might want to add a bunch of ,,, instead of the repeating whitespace in the center (a comma is treated as a regular whitespace by Clojure) • repeated (repeat 8 [x y]) - no need to hardcode that 8 in there, map stops on the shortest collection. I would also inline that usage, IMO it doesn't need a separate name • neighbors (map #(map + %1 %2) repeated directions) - here, you turn "point vectors" into "point lazy collections". Probably not a big deal at all here, but I'd still replace the inner map with mapv • (-> grid (nth x) (nth y)) - this pattern, with various indentations/spellings/contexts , is repeated a few times, I'd move it into a separate get-cell function • is-bomb? returns a number. It would be better if it returned a boolean and the code that uses it would then (count (filter identity ...)) or something like that • Names like is-bomb? and :is-open? are an antipattern, I'm pretty sure. I think there's a note on it somewhere in the community-driven Clojure Style Guide (although I myself don't agree with everything in that guide). In any case, I myself would write it as just bomb? and open? • (rf/reg-sub :grid ...) could be written shorter: (rf/reg-sub :grid :-> :grid). More details are in the docstring of rf/reg-sub • Simple events that do nothing but assoc'ing a thing into app-db might deserve their own shorthand. I would also suggest for any project that's not a quick test of something to copy around a wrapper of re-frame that you wrote yourself and can adapt to your own needs. Makes a lot of things smoother down the road. E.g. in my own wrapper I make it so that event IDs are never passed to event/sub handlers, so I don't have to write _ everywhere; I have more advanced shorthands for reg-sub than the ones built into re-frame, like that :->; and so on • Lines 105-106 - the formatting is confusing and makes the reader count the parens. I'd move lined to the next line and probably even that % as well. Also, since it's a conditional assoc, I'd replace the whole if in there with cond-> • In the cond inside click-handler you call dispatch in every branch. Maybe it's worth it to move that dispatch outside of cond and make the latter return the event vector • :key (str r "x" c) - you don't have to make strings yourself, if you want you can use :key [r c] or something like that • :width "20px" :height "20px" - when dealing with pixels in :style in Reagent, there's no need to specify the unit and use a string, you can simply say 20 instead of "20px". But some folks prefer when it's explicit • :border-style "solid" - similarly, "solid" can be :solid if you prefer that • (if (:is-open? cell) (:content cell)) - it's not a rare suggestion to replace single-branched ifs with whens • In games.core, you have r/with-let that's indented as a function. There's probably a way in your IDE to make it be indented as a regular let • Stuff like :section.section>div.container>div.content - of course YMMV and it might be a matter of preference, but after years of using that style I stopped and pretty much never use those shorthands anymore. It's easier to write but harder to refactor, to extend, to see nesting at a glance, to search for keywords

thanks a lot @p-himik for taking the time to review