Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement :while, :when-some #13

Open
wants to merge 4 commits into
base: v2.0.1
Choose a base branch
from
Open

Implement :while, :when-some #13

wants to merge 4 commits into from

Conversation

vemv
Copy link

@vemv vemv commented May 26, 2018

Implements :while (my whim), :when-some (#9).

Also adds a modest test suite and fixes an indentation that seemed off.

Hope it's all good! If yes, the next step would be consolidating the two src files into a single .cljc one, with the goodies added.

Victor

@vemv
Copy link
Author

vemv commented Jul 12, 2018

Hey there,

any luck with this one?

Thanks - V

@Engelberg
Copy link
Owner

I'm all for when-some, but when you added while, it raised a philosophical question in my mind as to whether this particular cond library could risk becoming a "kitchen sink" of so many possibilities that people have trouble remembering them all. For example, when adapting this from cgrand's original version of this, I removed his version of if-let because it had a different syntax than the other possibilities and seemed less useful in practice. I question the value of while here, because it is only useful in certain kinds of side-effecting code that would seem to me to rarely arise in the context of a cond. Have you found while actively useful inside of cond, or did you just put it in on a whim (as you described it in the pull request). If the latter, I would rather drop that from the pull request. So this is why I didn't respond quickly to the pull request; I was mulling this over without really coming to a decisive conclusion.

Another small point is that when writing library code I always prefer efficiency over concision. So, for example in your new:

(if (#{:while 'while} test)

I had some concern that this is less efficient than the or-based code, because it may cause each invocation of the macro to build up this small data structure, which takes some steps to create, and then it will later be garbage-collected (depends on whether the compiler is smart enough to lift this constant data structure outside of the lambda, but even then it will inherently be less efficient, because small set tests expand to the or code, but with the extra overhead of creating the data structure). I generally don't worry about these things in my own code, but for a library I feel that it is worth thinking about because I imagine a few cycles of efficiency multiplied across potentially many people's code.

So, if you can resubmit with just when-some (please put the when-some test further down in the sequence of tests -- I'd rather have :let, which I expect to be the most common thing, to be up towards the top for efficiency), and remove the change to set based testing on keywords, I'll merge it tonight. If you really want while, I'd like to hear more that it has been actively valuable to you.

Thanks again for submitting the pull request, and I apologize for my delay in responding.

@vemv
Copy link
Author

vemv commented Aug 7, 2018

Hi @Engelberg !

Thanks for the reply. Sorry as well for the delay, I also was ruminating on an optimal solution for this.

Probably a pretty fundamental difference in philosophies is that I don't want to use better-cond as a 'cond'. I'm just interested in how it can reduce nesting levels.

IOW, some things may make sense for a general 'syntax macro' like -> but not much for a macro like better-cond.

Also, it's not that good to have a whitelist of clauses (:while, :when-some) because the implementation indeed gets kitchen-sinky, and also disagreements (like the one around :while) are not desirable.

With those considerations in mind I wrote the following macro:

(defmacro with [clauses & body]
  (let [[test expr & more-clauses] (seq clauses)
        test (if (symbol? test)
               test
               (-> test str (subs 1) symbol))]
    (if more-clauses
      `(~test ~expr (with ~more-clauses (do ~@body)))
      `(~test ~expr ~@body))))

Example use:

  (with [:let [a 2]
         :when-let [b (some-> a (* 2))]
         :for [i (range 10)]]
        (println i)
        [b i])

The nice thing is that it accept arbitrary clauses, and accordingly the behavior can be extended for user-provided macros.

My with doesn't have dependencies on better-cond but we could add it anyway as it seems closely-related.

WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants