Can someome review my code : https://github.com/RoelofWobben/Learning_in_public/blob/main/ground_up/src/ground_up/chapter5.clj
a small thing: :false
and :true
should probably be false
and true
- then your macro can just be (if logging-enabled
(prn ~message))`
the nil
is implicit, and message
needs to be unquoted
in the style of your previous macro, (prn ~message)` could also be
(list
prn message)` the two are equivalent
@noisesmith thanks, I see now only a the message (prn message)
and not (prn :hi)
as the challenge said the outcome should be
and a message that the else branch is missing
wait, who says an else branch is missing?
it's optional
here
I disagree with clj-kondo, but it's clj-kondo (a style tool) telling you this
Use when
instead of if
when you need to return nil
from else branch.
this is a controversial suggestion - many think that when
is implicitly for side effects (I could go either way, but wouldn't go far as saying all one armed ifs should be replaced with when - if allows a single arm for a reason)
I have never seen using when
just for side effects. I just use when
instead of (if test branch nil)
Or maybe I just have got used to it because I use kibit
https://github.com/jonase/kibit
I usually use (if test branch)
or (and test then)
for values, I definitely never use (if test branch nil)
When I see (if test branch)
I have feeling that the author forgot to add else
😄
it's harmless to add a nil
else branch, but it is also redundant
(ins)user=> (if false :OK)
nil
(ins)user=> (if false :OK nil)
nil
it runs so I do not have a problem with it
the only thing that worries me that I see prn message
as output instead of (pnr :hi)
as stated in the challenge
your original will show that:
(defmacro log [message]
(if (= logging-enabled :true)
`(prn message)
nil))
my version won't:
(defmacro log [message]
(if logging-enabled
`(prn ~message)))
I think you skipped part of my feedbacknotice the ~
to unquote message
change the if
to a when
to mollify clj-kondo
or and
if you think when is only for side effects and don't mind a random false instead of nil :D
@noisesmith then I see (prn ~ message)
as output
are you sure you aren't using ' instead of ` ?
the ~ should not show up in the macro expand, it's a special instruction to the ` reader macro
yes, im sure
(def logging-enabled true)
(defmacro log [message]
(if logging-enabled
'(prn ~message)))
(macroexpand `(log :hi))
that's not `, it's '
it's the wrong one
oke, this code works
thanks for the feedback
glad I could help