This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2024-06-02
Channels
- # announcements (11)
- # babashka (3)
- # beginners (54)
- # cherry (1)
- # cider (11)
- # clojure (28)
- # clojure-europe (1)
- # clojure-norway (4)
- # clojure-sweden (3)
- # clojurescript (40)
- # community-development (3)
- # cursive (12)
- # datomic (2)
- # emacs (6)
- # hyperfiddle (12)
- # missionary (14)
- # off-topic (13)
- # overtone (10)
- # re-frame (13)
- # releases (1)
- # ring (2)
- # specter (11)
- # tools-deps (7)
hi. i'm practicing reframe so i wrote this small minesweeper game using it. here is the code https://github.com/bibiki/games/blob/main/src/cljs/games/pages/minesweeper.cljs
it'd be nice if somebody with experience using reframe looked at it and gave any feedback. one thing i am in particular interested to know is this:
i have a start button to initiate a new layout of mines. however, initially, i wanted the layout to be initiated immediately as the pages is accessed. for that purpose, i had a line that said (rf/dispatch [:gridalize])
I could see the relevant reg-event-db was running as i had a println inside it, but my grid of mines would still come null inside my game function where i subscribe to the value of grid. I could not figure out why that was, so I just added the start button to have more control over that. if somebody knows how I could have done that, I d be happy if you'd shared that with m e
Also, do you want feedback just about re-frame or also about CLJ/S or even programming in general?
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."
yeah, that did work. thanks @U2FRKM4TW. 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 if
s with when
s
• 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