code-reviews

2020-12-11T16:34:49.081600Z

a small thing: :false and :true should probably be false and true - then your macro can just be (if logging-enabled (prn ~message))`

2020-12-11T16:35:03.082Z

the nil is implicit, and message needs to be unquoted

2020-12-11T16:36:01.082800Z

in the style of your previous macro, (prn ~message)` could also be (list prn message)` the two are equivalent

roelof 2020-12-11T17:20:44.084400Z

@noisesmith thanks, I see now only a the message (prn message) and not (prn :hi) as the challenge said the outcome should be

roelof 2020-12-11T17:21:02.084900Z

and a message that the else branch is missing

2020-12-11T17:26:42.085200Z

wait, who says an else branch is missing?

2020-12-11T17:26:45.085400Z

it's optional

roelof 2020-12-11T17:28:10.085600Z

here

roelof 2020-12-11T17:28:14.085700Z

2020-12-11T17:29:59.086300Z

I disagree with clj-kondo, but it's clj-kondo (a style tool) telling you this

lukas.rychtecky 2020-12-14T08:38:02.099200Z

Use when instead of if when you need to return nil from else branch.

2020-12-14T16:28:29.101700Z

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)

lukas.rychtecky 2020-12-14T17:05:30.101900Z

I have never seen using when just for side effects. I just use when instead of (if test branch nil)

lukas.rychtecky 2020-12-14T17:06:43.102100Z

Or maybe I just have got used to it because I use kibithttps://github.com/jonase/kibit

2020-12-14T17:12:44.102400Z

I usually use (if test branch) or (and test then) for values, I definitely never use (if test branch nil)

lukas.rychtecky 2020-12-15T08:29:18.102600Z

When I see (if test branch) I have feeling that the author forgot to add else 😄

2020-12-11T17:30:15.086700Z

it's harmless to add a nil else branch, but it is also redundant

2020-12-11T17:30:41.087Z

(ins)user=> (if false :OK)
nil
(ins)user=> (if false :OK nil)
nil

roelof 2020-12-11T17:33:42.087500Z

it runs so I do not have a problem with it

roelof 2020-12-11T17:34:34.088600Z

the only thing that worries me that I see prn message as output instead of (pnr :hi) as stated in the challenge

2020-12-11T17:38:17.090100Z

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 feedback

2020-12-11T17:38:36.090400Z

notice the ~ to unquote message

nate 2020-12-11T17:39:06.090900Z

change the if to a when to mollify clj-kondo

2020-12-11T17:39:58.091500Z

or and if you think when is only for side effects and don't mind a random false instead of nil :D

roelof 2020-12-11T17:43:16.092Z

@noisesmith then I see (prn ~ message) as output

2020-12-11T17:44:55.092300Z

are you sure you aren't using ' instead of ` ?

2020-12-11T17:45:31.092700Z

the ~ should not show up in the macro expand, it's a special instruction to the ` reader macro

roelof 2020-12-11T17:47:10.092900Z

yes, im sure

roelof 2020-12-11T17:47:14.093200Z

(def logging-enabled true)

(defmacro log [message]
  (if logging-enabled
    '(prn ~message)))


(macroexpand `(log :hi))

2020-12-11T17:48:24.093500Z

that's not `, it's '

2020-12-11T17:48:25.093700Z

it's the wrong one

roelof 2020-12-11T17:54:04.094Z

oke, this code works

roelof 2020-12-11T17:54:11.094200Z

roelof 2020-12-11T17:54:27.094700Z

thanks for the feedback

2020-12-11T17:55:57.094900Z

glad I could help