Translating Java code with nested for loops

62 views
Skip to first unread message

Bhinderwala, Shoeb

unread,
Jun 28, 2011, 8:20:59 PM6/28/11
to clo...@googlegroups.com

Hi

I have been learning clojure for some time but am a bit stumped when translating code with nested for loops.

Can someone help me to translate the following java code to clojure elegantly:

The inputs are two arrays of type double of the same length dailyValues and totalValues. The output is the array contrib of the same length.

        int n = dailyValues.length;

        for (int i = 0; i < n; i++)

        {

            sum = 1.0;

            for (int j = i + 1; j < n; j++)

            {

                sum *= (1.0 + (totalValues[j] / 100.0));

            }

            contrib[i] = sum * dailyValues[i];

        }

Many thanks for your help.

-- Shoeb

David Sletten

unread,
Jun 28, 2011, 11:37:35 PM6/28/11
to clo...@googlegroups.com

On Jun 28, 2011, at 8:20 PM, Bhinderwala, Shoeb wrote:

The inputs are two arrays of type double of the same length dailyValues and totalValues. The output is the array contrib of the same length.


        int n = dailyValues.length;

        for (int i = 0; i < n; i++)

        {

            sum = 1.0;

            for (int j = i + 1; j < n; j++)

            {

                sum *= (1.0 + (totalValues[j] / 100.0));

            }

            contrib[i] = sum * dailyValues[i];

        }


1. Are you sure that this does what you want it to?
2. What does it do?

There are two obvious red flags with the code:
-You have a variable called 'sum' which is really a product.
-You never use index 0 of totalValues

Have all good days,
David Sletten




Justin Kramer

unread,
Jun 29, 2011, 1:11:46 AM6/29/11
to Clojure
Here's one way.

(defn tails [coll]
(take-while seq (iterate rest coll)))

(defn calc [total-values daily-values]
(map * daily-values (for [tail (tails total-values)]
(reduce #(* %1 (inc (/ %2 100.0)))
1.0
(rest tail)))))

In translating it, I first tried to visualize the algorithm[1]. Then I
transcribed that visualization into the usual suspects: map/for,
reduce, filter. Having a solid grasp of each of those -- not to
mention the rest of clojure.core -- is very helpful.

[1] http://i.imgur.com/XDZhm.png (crude drawing of the first step)

Hope that helps,

Justin

On Jun 28, 8:20 pm, "Bhinderwala, Shoeb"
<SABhinderw...@wellington.com> wrote:
> Hi
>
> I have been learning clojure for some time but am a bit stumped when
> translating code with nested for loops.
>
> Can someone help me to translate the following java code to clojure
> elegantly:
>
> The inputs are two arrays of type double of the same length -

David Sletten

unread,
Jun 29, 2011, 1:45:53 AM6/29/11
to clo...@googlegroups.com
Assuming that your code is correct, the following reproduces your results:
(defn compute-contrib [daily-values total-values]
  (loop [contrib []
         daily-values daily-values
         total-values total-values]
    (if (empty? daily-values)
      contrib
      (recur (conj contrib (* (first daily-values) (reduce (fn [sum total-value] (* sum (+ (/ total-value 100.0) 1.0)))
                                                           1.0
                                                           (rest total-values))))
             (rest daily-values)
             (rest total-values)))) )

In the outer loop you are working with each element of dailyValues and consecutively smaller subsequences of totalValues. In the inner loop you repeat a calculation using each of the remaining elements of totalValues starting with a seed value of 1.0.

The Clojure implementation uses 'loop' to traverse each of the input sequences. On each iteration we use 'reduce' to do the work of the inner loop (notice that 'reduce' is working with (rest total-values) rather than simply total-values itself, so we discard the first element each time just as your inner loop does), multiply that by the current element of interest in daily-values and then accumulate the result by conjoining it with our result 'contrib'.

The mental shift that you need to make involves forgetting about the index variables i and j and thinking instead about what the loops are accomplishing. If an inner loop is collapsing a sequence to a single result as above, then 'reduce' is what you want to use. Other inner loops might require 'map' or 'filter' depending on the computation.

Meikel Brandmeyer

unread,
Jun 29, 2011, 2:02:31 AM6/29/11
to clo...@googlegroups.com
Hi,

just for fun another variation. The two reverse calls are ugly, but necessary to get the order right for the reductions call.

(defn contribution
  [daily-values total-values]
  (let [weights (->> (rest total-values)
                  (map #(/ % 100.0))
                  reverse
                  (reductions #(* %1 (inc %2)) 1.0)
                  reverse)]
    (map * daily-values weights)))

Sincerely
Meikel

David Sletten

unread,
Jun 29, 2011, 2:02:23 AM6/29/11
to clo...@googlegroups.com

On Jun 29, 2011, at 1:45 AM, David Sletten wrote:

(defn compute-contrib [daily-values total-values]
  (loop [contrib []
         daily-values daily-values
         total-values total-values]
    (if (empty? daily-values)
      contrib
      (recur (conj contrib (* (first daily-values) (reduce (fn [sum total-value] (* sum (+ (/ total-value 100.0) 1.0)))
                                                           1.0
                                                           (rest total-values))))
             (rest daily-values)
             (rest total-values)))) )


Arrgghh! Now you've got me doing it too... :)

Replace 'sum' with 'product':
(defn compute-contrib [daily-values total-values]
  (loop [contrib []
         daily-values daily-values
         total-values total-values]
    (if (empty? daily-values)
      contrib
      (recur (conj contrib (* (first daily-values) (reduce (fn [product total-value] (* product (+ (/ total-value 100.0) 1.0)))
                                                           1.0
                                                           (rest total-values))))
             (rest daily-values)
             (rest total-values)))) )

hci

unread,
Jun 29, 2011, 2:03:09 AM6/29/11
to Clojure
How about this one?

(defn calc [total-values daily-values]
(map-indexed
(fn [i daily]
(* daily
(reduce #(* %1 (inc (/ %2 100.0))) 1.0 (nthnext total-
values (inc i)))))
daily-values))

Happy coding.

-huahai

On Jun 28, 10:11 pm, Justin Kramer <jkkra...@gmail.com> wrote:
> Here's one way.
>
> (defn tails [coll]
>   (take-while seq (iterate rest coll)))
>
> (defn calc [total-values daily-values]
>   (map * daily-values (for [tail (tails total-values)]
>                         (reduce #(* %1 (inc (/ %2 100.0)))
>                                 1.0
>                                 (rest tail)))))
>
> In translating it, I first tried to visualize the algorithm[1]. Then I
> transcribed that visualization into the usual suspects: map/for,
> reduce, filter. Having a solid grasp of each of those -- not to
> mention the rest of clojure.core -- is very helpful.
>
> [1]http://i.imgur.com/XDZhm.png(crude drawing of the first step)

Alan Malloy

unread,
Jun 29, 2011, 3:53:50 AM6/29/11
to Clojure
If you're already doing a bunch of multiplication in the reduce, you
don't need to do it outside as well, do you?

And because reduce doesn't care about laziness, you can use drop
instead of nthnext (which I think is clearer in almost all cases).

(defn calc [total-values daily-values]
(map-indexed
(fn [i daily]
(reduce #(* %1 (inc (/ %2 100.0)))
daily
(drop (inc i) total-values)))
daily-values))

And I'm not sure, but I think it's clearer what's going on if you pull
the manipulation of the total-values into a separate map, and just
reduce with a simple multiplication:

(defn calc [total-values daily-values]
(map-indexed
(fn [i daily]
(reduce * daily
(map #(inc (/ % 100.0))
(drop (inc i) total-values))))
daily-values))

And once you've done that, you can pretty simply reduce the amount of
nesting to define this as a threaded pipeline of operations starting
with total-values:

(defn calc [total-values daily-values]
(map-indexed
(fn [i daily]
(->> total-values
(drop (inc i))
(map #(inc (/ % 100.0)))
(reduce * daily)))
daily-values))

This layout makes it fairly clear that the first element of total-
values is never being used, but it would be nice if we could say so
explicitly. So finally, we can wrap the whole thing in a let:

(defn calc [total-values daily-values]
(let [interesting-values (next total-values)]
(map-indexed
(fn [i daily]
(->> total-values
(drop i)
(map #(inc (/ % 100.0)))
(reduce * daily)))
daily-values)))

On Jun 28, 11:03 pm, hci <huahai.y...@gmail.com> wrote:
> How about this one?
>
> (defn calc [total-values daily-values]
>   (map-indexed
>      (fn [i daily]
>         (* daily
>            (reduce #(* %1 (inc (/ %2 100.0))) 1.0 (nthnext total-
> values (inc i)))))
>      daily-values))
>
> Happy coding.
>
> -huahai
>
> On Jun 28, 10:11 pm, Justin Kramer <jkkra...@gmail.com> wrote:
>
>
>
>
>
>
>
> > Here's one way.
>
> > (defn tails [coll]
> >   (take-while seq (iterate rest coll)))
>
> > (defn calc [total-values daily-values]
> >   (map * daily-values (for [tail (tails total-values)]
> >                         (reduce #(* %1 (inc (/ %2 100.0)))
> >                                 1.0
> >                                 (rest tail)))))
>
> > In translating it, I first tried to visualize the algorithm[1]. Then I
> > transcribed that visualization into the usual suspects: map/for,
> > reduce, filter. Having a solid grasp of each of those -- not to
> > mention the rest of clojure.core -- is very helpful.
>
> > [1]http://i.imgur.com/XDZhm.png(crudedrawing of the first step)

Ken Wesson

unread,
Jun 29, 2011, 3:55:52 AM6/29/11
to clo...@googlegroups.com
On Wed, Jun 29, 2011 at 3:53 AM, Alan Malloy <al...@malloys.org> wrote:
> This layout makes it fairly clear that the first element of total-
> values is never being used, but it would be nice if we could say so
> explicitly. So finally, we can wrap the whole thing in a let:
>
> (defn calc [total-values daily-values]
>  (let [interesting-values (next total-values)]
>    (map-indexed
>     (fn [i daily]
>       (->> total-values
>            (drop i)
>            (map #(inc (/ % 100.0)))
>            (reduce * daily)))
>     daily-values)))

Oops.

--
Protege: What is this seething mass of parentheses?!
Master: Your father's Lisp REPL. This is the language of a true
hacker. Not as clumsy or random as C++; a language for a more
civilized age.

Alan Malloy

unread,
Jun 29, 2011, 4:50:23 AM6/29/11
to Clojure
On Jun 29, 12:55 am, Ken Wesson <kwess...@gmail.com> wrote:
> On Wed, Jun 29, 2011 at 3:53 AM, Alan Malloy <a...@malloys.org> wrote:
> > This layout makes it fairly clear that the first element of total-
> > values is never being used, but it would be nice if we could say so
> > explicitly. So finally, we can wrap the whole thing in a let:
>
> > (defn calc [total-values daily-values]
> >  (let [interesting-values (next total-values)]
> >    (map-indexed
> >     (fn [i daily]
> >       (->> total-values
> >            (drop i)
> >            (map #(inc (/ % 100.0)))
> >            (reduce * daily)))
> >     daily-values)))
>
> Oops.

Not sure if this is what you're pointing out (a single word is tricky
to decipher), but I did forget to *use* interesting-values after
defining it: it should be the first form in the ->> list.

Huahai Yang

unread,
Jun 29, 2011, 11:22:11 AM6/29/11
to Clojure
The multiplication outside the reduce is required because it is part
of the original logic: "contrib[i] = sum * dailyValues[i];". Using the
drop function is a very good call though.

-huahai

On Jun 29, 12:53 am, Alan Malloy <a...@malloys.org> wrote:
> If you're already doing a bunch of multiplication in the reduce, you
> don't need to do it outside as well, do you?
>
> And because reduce doesn't care about laziness, you can use drop
> instead of nthnext (which I think is clearer in almost all cases).
>
> (defn calc [total-values daily-values]
>   (map-indexed
>      (fn [i daily]
>        (reduce #(* %1 (inc (/ %2 100.0)))
>                daily
>                (drop (inc i) total-values)))
>      daily-values))
>


Bhinderwala, Shoeb

unread,
Jun 29, 2011, 1:15:16 PM6/29/11
to clo...@googlegroups.com
Thanks to all for the excellent and quick responses.

David - you are right the variable should be called product not sum. This is just some poorly written Java code that I am translating to Clojure.

Justin - thanks for the idea on how to visualize such algorithms and the picture you shared.

--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to
clojure+u...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en

Alan Malloy

unread,
Jun 29, 2011, 9:39:42 PM6/29/11
to Clojure
(* daily (reduce * 1 coll))
;; is exactly equal to
(reduce * daily coll)

My point is you can start with daily instead of with 1, and thus not
have to special-case it in at the end.

Huahai Yang

unread,
Jun 30, 2011, 12:32:59 AM6/30/11
to Clojure
Missed that one. Yeah, that's much nicer. -huahai

Bhinderwala, Shoeb

unread,
Jun 30, 2011, 12:48:35 AM6/30/11
to clo...@googlegroups.com
Hi Alan

I really liked the way you kept refactoring the original solution by Huahai to make it more and more logical and elegant. I verified that each refactored function produced the correct results.

Thanks for sharing your valuable thoughts and techniques.

Shoeb

-----Original Message-----
From: clo...@googlegroups.com [mailto:clo...@googlegroups.com] On Behalf Of Huahai Yang
Sent: Thursday, June 30, 2011 12:33 AM
To: Clojure
Subject: Re: Translating Java code with nested for loops

--

Reply all
Reply to author
Forward
0 new messages