2
\$\begingroup\$

I'm starting to learn Clojure and as an exercise I've implemented the following functions (written in Java) in Clojure:

private int[] SwapValues(int val1, int val2, int[] array) {
    int[] result = new int[array.length];
    for (int i = 0; i < array.length; i++) {
        if (array[i] == val1) {
            result[i] = val2;
        } else if(array[i] == val2) {
            result[i] = val1;
        } else {
            result[i] = array[i];
        }
    }

    return result;
}


public int[][] PartialCross(int[] subject1, int[] subject2, final int begin, final int end) {
    int[][] result = new int[2][subject1.length];
    int[] child1 = subject1.clone();
    int[] child2 = subject2.clone();

    for (int i = begin; i < end; i++) {
        child1 = SwapValues(subject1[i], subject2[i], child1);
        child2 = SwapValues(subject1[i], subject2[i], child2);
    }

    result[0] = child1;
    result[1] = child2;

    return result;
}

My result is:

(def chromosone1 [5 8 1 7 6 4 9 3 2])
(def chromosone2 [5 3 2 6 1 8 7 4 9])

(defn extract [input i j]
  (filter #(not (nil? %1)) 
          (map-indexed #(if (and (>= %1 i) (< %1 j)) %2) input)))

(defn chrossover-swap [input i]
  (let [i1 (first i)
        i2 (last i)]
    (map #(cond
           (= %1 i1) i2
           (= %1 i2) i1
           :else %1) input)))

(defn crossover [input1 input2]
  (let [i1 (extract input1 3 5)
        i2 (extract input2 3 5)
        i3 (partition 2 (interleave i1 i2))]
    (loop [to-swap i3
           o1 input1
           o2 input2]
      (if (zero? (count to-swap))
        [o1 o2]
        (do
          (def i (first to-swap))
          (recur (rest to-swap) (chrossover-swap o1 i) (chrossover-swap o2 i)))))))

(print (crossover chromosone1 chromosone2))

The code works, but I may have made it more complicated than it needs to be. I'm a beginner in functional programming and would like to know if this could be written in a simpler or more efficient way.

\$\endgroup\$
  • \$\begingroup\$ Hi! Welcome to Code Review. Good job on your first post! \$\endgroup\$ – TheCoffeeCup Nov 7 '15 at 1:28
2
\$\begingroup\$

looks great!

(filter #(not (nil? %1)) ...)

Prefer % instead of %1 for single arguments

Simpler to write as (remove nil? ...)

(map #(cond
       (= %1 i1) i2
       (= %1 i2) i1
       :else %1) input))

When you have map + function definition in one statement, consider creating a named function with a defn, or using a for expression:

(for [i input]
  (cond ...))

Never ever def inside a loop:

(do
  (def i (first to-swap))

def creates a global var http://clojure.org/vars

You want:

(let [i (first to-swap)]
  ...)

Consider using destructuring:

(loop [[i & more] i3
  ...
 (if i
   (recur more (swap o1 i) (swap o2 i))
   [o1 o2]))
\$\endgroup\$
2
\$\begingroup\$

Further to @TimothyPratley's answer:

Functional programming enables you to develop functions that express common computing patterns. Many (most?) such functions are in the standard Clojure core library. Two of these, replace and reduce, turn out to be helpful here:

  • You can use replace to define chrossover-swap.
  • The idiomatic way to express crossover is to use reduce.

Before we do this, let's note that your extract function is just the standard subvec, so it disappears.

So your code becomes ...

(defn chrossover-swap [input [i1 i2]]
  (replace {i1 i2, i2 i1} input))

(defn crossover [input1 input2]
  (let [i1 (subvec input1 3 5)
        i2 (subvec input2 3 5)
        i3 (map vector i1 i2)]
    (reduce
      (fn [[o1 o2] i] [(chrossover-swap o1 i) (chrossover-swap o2 i)])
      [input1 input2]
      i3)))

The reduce in the crossover function chews through the sequence i3, using each element to yield a new [o1 o2]. Its arguments are

  • the function applied to [o1 o2] and sequence element i to yield the new [o1 o2];
  • [input1 input2] - the initial [o1 o2];
  • i3 - the sequence whose elements reduce feeds to the function in turn.

Note

  • Your loop may well be faster than this reduce.
  • map can take several sequence arguments, which are fed in parallel to the function. Thus (map vector i1 i2) is more direct than (partition 2 (interleave i1 i2)).

Having said this, you'd probably want to define crossover to accept the subrange indices as arguments. They won't always be 3 and 5, I suppose.

So we define

(defn crossover [input1 input2 [start end]]
  (let [i1 (subvec input1 start end)
        i2 (subvec input2 start end)
        i3 (map vector i1 i2)]
    (reduce
      (fn [[o1 o2] i] [(chrossover-swap o1 i) (chrossover-swap o2 i)])
      [input1 input2]
      i3)))

And call it thus:

(print (crossover chromosone1 chromosone2 [3 5]))

... to get the same effect as before.

\$\endgroup\$
  • \$\begingroup\$ Wow, this is great! It's almost half of the original code :). Thank you! \$\endgroup\$ – ionutt93 Nov 26 '15 at 20:02

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.