0
\$\begingroup\$

I am a beginner to Clojure and I made a little script that prints out every prime number from 1 to 1000. Let me know if there are any improvements that I can make:

(loop [i 2]
  (loop [j 2]
    (when (= j i)
      (println i))
    (when-not (= (mod i j) 0)
          (recur (inc j))))
  (when (< i 1000)
    (recur (inc i))))
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Note: It has been a long time since I actively played with Clojure, so I will mostly refrain from writing and/or reviewing any actual code in my answer and will only provide a high-level review.

Separating I/O from computation

In your program, you are mixing I/O (printing) and computation. This is generally a bad idea, not just in Clojure, even not just in Functional Programming, but in Programming in general.

  • It makes it hard to automatically test your program: you have to run the entire program, somehow capture the console output, and compare the console output to a previously recorded run.
  • It makes it hard to reuse your program: what if I don't want to print the primes to the console but to a file? What if I want to surround every prime with parentheses? What if I want to display them on a webpage? Store them in a database? Use them as the datasource in a dropdown menu?

What you should do instead is to separate the printing from the computing. Have one function which computes the list of primes and another function which prints the list.

In fact, once you do this, you will notice another advantage of making things more general: not only is your code easier to reuse for other people, but also, you can reuse other people's code! In particular, printing a list of prime numbers is a very specialized operation, so it is highly unlikely that there is a ready-made function for you to reuse. But, printing a list on the other hand is a very common operation, and indeed, there already exists a function which can do that.

So, when I wrote above that you should have two functions, one which generates the list and which prints it, that was actually wrong: you only need the function which generates the list, the function for printing it was already written for you by someone else.

Separating Generation, Transformation, Filtering, and Reduction of Data

Very similar to the arguments for separating I/O from computation, it also makes sense to separate different kinds of data manipulation from each other. Again, this is not specific to Clojure or even to Functional Programming, but is a general Programming Principle.

For example, what if I want the prime numbers less than 100 or less than 10000 instead of less than 1000? What if, instead of the prime numbers less than 1000 I want the first 1000 prime numbers?

In Functional Programming in particular, it is often preferred to generate an infinite stream of data and let the user decide how much of it and which of it to use. In Clojure, there exists an abstraction which can be used to represent infinite streams of data: the Sequence (seq).

In your case, I can see (at least) two possible approaches:

Approach #1

Split the code into three separate functions:

  1. Generate an infinite stream of all integers
  2. Filter on primality
  3. Take integers while they are smaller than 1000

This has the advantage of being very general, because I can independently replace all three of the parts. For example, I can generate a stream of only the odd integers, or maybe I am only interested in the prime numbers from some specific list of integers, not all of them.

It also has the advantage that there are already ready-made functions to use for all three parts. For example, clojure.core/range can generate an infinite seq of integers, clojure.core/filter can filter out elements of a seq based on a predicate, and clojure.core/take-while can take elements of a seq as long as a predicate is true.

So, the only two functions you have to write are the two predicates, where the latter one is just something like #(> 1000 %). Which means the only "real work" you have to do is to define the primality predicate.

The big disadvantage of this approach is that it is wasteful for the specific problem that you are solving: there are lots of integers which are not prime numbers, and all of them need to be generated.

This, by the way, is also often a general property of programming: if you make things more general, they become less optimized for the specific case.

(def integers (range))

; Taken from https://stackoverflow.com/a/48121239/2988
(defn prime? [n]
  (if (< 1 n)
    (empty? (filter #(= 0 (mod n %)) (range 2 n)))
    false))

(def primes (filter prime? integers))

(defn primes-below [n] (take-while #(> n %) primes))

(def primes-below-1000 (primes-below 1000))
(def primes-below-100 (primes-below 100))

(println primes-below-100)
; (2 3 5 7 11 13 17 19 23 29 31 37 41 43 47 53 59 61 67 71 73 79 83 89 97)

Note: as mentioned above, I haven't written Clojure in almost 15 years, so this is definitely not idiomatic Clojure code. It does, however, show the general approach.

A modern idiomatic Clojure implementation would probably use Transducers.

Approach #2

  1. Generate an infinite stream of all primes
  2. Take integers while they are smaller than 1000

This is less general than approach #1, but it still allows me to choose which primes to select.

The second function is the same one as the third function from above, i.e. we can use clojure.core/take-while for it. For the first function, Clojure also has some helpers for us, for example, both clojure.core/repeatedly and clojure.core/iterate can generate an infinite stream from a function that produces the next element.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.