Is this the good way to write get-percentage

[Cecil Westerhof]

I needed a function to get the percentage as an int. Input is place and total-count.
I want the normal definition (which is the default) and a high and low variant.

I came up with the following code:
    (defn get-percentage
      ([place total-count] (get-percentage :normal place total-count))
      ([mode place total-count]
        (let [percentage (/ (* place 100.0) total-count)]
          (condp = mode
            :high     (int (Math/ceil  percentage))
            :low      (int (Math/floor percentage))
            :normal   (int (Math/round percentage))
            (throw (Exception. "ERROR: get-percentage [:high|:low|:normal] <PLACE> <TOTAL_COUNT>"))))))

Is this a good version, or could it be done better?

--

Cecil Westerhof

[Michael Blume]

I think you could replace your condp = with case, since all your mode keywords are known at compile-time, otherwise looks about right.

--
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
---
You received this message because you are subscribed to the Google Groups "Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Matching Socks]

There was also some discussion of this back in November, where Fluid
Dynamics suggested higher-order functions, and tbc++, multimethods(!).

The main source of complexity in get-percentage has to do with
selecting what it should do.  Also, both percentage computation and
rounding could potentially be useful on their own without the other.
Those considerations recommend higher order functions.  Using
higher-order functions can make the code shorter, clearer, and more
flexible.

First, there's the essential computation:

(defn pct [p t] (/ (* p 100.0) t))

Second, various general-purpose ways to round floating-point numbers:

(defn round-up [x]   (int (Math/ceil x)))
(defn round-down [x] (int (Math/floor x)))
(defn round [x]      (int (Math/round x)))

To put them together, get-percentage took a hint about the rounding
function, dispatched on the hint, and took upon itself responsibility
for detecting a bogus hint.  Instead of a hint from a closed set, why
not let the caller pass in the rounding function itself?  Now
get-percentage is down to one line:

(defn get-percentage* [f p t] (f (pct p t)))

For example

(get-percentage* round-up 4 30)
14
(get-percentage* round-down 4 30)
13

Alternatively, the standard function "comp" can produce your canned
combinations, so you might not need get-percentage* itself anymore:

(def get-percentage-high (comp round-up pct))

For example:

(get-percentage-high 4 30)
14

[Jan-Paul Bultmann]

I really don't think that calculating percentages warrants a DSL.
(I'm not even sure if percent warrants a function ;} )

`(round (percent ...))
(round-up (percent ...))
(round-down (percent ...))`

Seems far more readable to me than having a function that does many things at once, be it higher order, keyword configured or pre `comp`ed.

Cheers Jan

[Jonathan Winandy]

Hi ! 

You could rewrite the code like that :  

    (defn get-percentage

      ([place total-count] (get-percentage :normal place total-count))

      ([mode place total-count]

    

       (let [mode-fn (case mode

                           :high    Math/ceil  

                           :low     Math/floor 

                           :normal  Math/round 

                           (throw 

(Exception. "ERROR: get-percentage [:high|:low|:normal] <PLACE> <TOTAL_COUNT>")))]

       

         (-> place (* 100.0) (/ total-count) mode-fn int))))

I don't know if it's really better, it just removes duplicated code.

You can separate the function in two : 

(defn to-mode-fn [mode]

  (if (keyword? mode)

    (case mode :high    Math/ceil  

               :low     Math/floor 

               :normal  Math/round

      (throw  (Exception. "ERROR: get-percentage [:high|:low|:normal] <PLACE> <TOTAL_COUNT>")))

    mode))

(defn get-percentage

  ([place total-count] (get-percentage :normal place total-count))

  ([mode-fn place total-count]

     (-> place (* 100.) (/ total-count) (to-mode-fn mode-fn) int))) 

;usage 

(get-percentage :normal 10 100)

(get-percentage Math/floor 10 100)

Have a nice day, 

Jon

[Cecil Westerhof]

​Everyone thanks for the comments. I did the following.

I made three round functions:

    (defn round      [x] (int (Math/round x)))

    (defn round-high [x] (int (Math/ceil  x)))
    (defn round-low  [x] (int (Math/floor x)))

And rewrote get-percentage to:

    (defn get-percentage
      ([place total-count] (get-percentage :normal place total-count))
      ([mode place total-count]
   

      (let [mode-fn (case mode
                      :high    round-high
                      :low     round-low
                      :normal  round
                      (throw (Exception.

                              "ERROR: get-percentage [:high|:low|:normal] <PLACE> <TOTAL_COUNT>")))]
        (-> place (* 100.0) (/ total-count) mode-fn int))))

But I wanted to do it a little neater.

I wrote a new round function:
    (defn round
      ([x] (round :normal x))
      ([mode x]
        (println mode)
        (println x)
        (case mode
          :high    (Math/ceil  x)
          :low     (Math/floor x)
          :normal  (Math/round x)
          (throw (Exception.
                  "ERROR: round [:high|:low|:normal] <VALUE>")))))

Then I tried to rewrite get-percentage:

    (defn get-percentage
      ([place total-count] (get-percentage :normal place total-count))
      ([mode place total-count]
   

        (if-not (contains? #{:high :low :normal} mode)

                (throw (Exception.
                        "ERROR: get-percentage [:high|:low|:normal] <PLACE> <TOTAL_COUNT>")))

        (round mode ​(/ (* place 100.0) total-count))))

But this gives on the last line:
    CompilerException java.lang.RuntimeException: Unable to resolve symbol: ​ in this context,

When I put before that statement:

​    (println mode)
    (println place)
    (println total-count)
    (println ​(/ place total-count))
    (println ​(/ (* place 100.0) total-count))

I get the same error on:
    (println ​(/ place total-count))

What is happening here?

I am using Clojure 1.6.0 on Linux.​

--

Cecil Westerhof

[Cecil Westerhof]

2015-02-13 11:30 GMT+01:00 Jonathan Winandy <jonathan...@gmail.com>:

(defn to-mode-fn [mode]

  (if (keyword? mode)

    (case mode :high    Math/ceil  

               :low     Math/floor 

               :normal  Math/round

      (throw  (Exception. "ERROR: get-percentage [:high|:low|:normal] <PLACE> <TOTAL_COUNT>")))

    mode))

 

​If I execute that I get:
    CompilerException java.lang.RuntimeException: Unable to find static field: ceil in class java.lang.Math, compiling:(NO_SOURCE_PATH:3:5)

I am using clojure 1.6.0 on Linux.​

--

Cecil Westerhof

[Matching Socks]

A Clojure fn is an Object, but Math's ceil method is not an Object.  The Java-Interop notation for static methods, eg (Math/ceil...), papers over this difference.  If you want to manipulate something like Math/ceil as a Clojure fn, you can wrap it in one such as your round-high.

http://stackoverflow.com/questions/8623109/how-to-get-a-clojure-handle-on-a-java-static-method-similar-to-memfn-for-a-ja

http://clojure.org/java_interop

P.S. If you find this error message puzzling and think the compiler could reasonably do better, would you file a Jira ticket about it?  http://dev.clojure.org/jira/

[Jonathan Winandy]

"Unable to resolve symbol: ​ in this " 

I get this error when I have a non-breaking space in my code : 

http://en.wikipedia.org/wiki/Non-breaking_space

Jon

--

[Cecil Westerhof]

2015-02-14 15:46 GMT+01:00 Jonathan Winandy <jonathan...@gmail.com>:

"Unable to resolve symbol: ​ in this " 

I get this error when I have a non-breaking space in my code : 

http://en.wikipedia.org/wiki/Non-breaking_space

​It was not a non-breaking space (those I see), but something like it, with code #x200b. After removing these characters it worked.

--

Cecil Westerhof

[Cecil Westerhof]

​I have now something that is working OK I think. This is what it has become:​

​    (defmacro static-fn [f] `(fn [x#] (~f x#)))

    (defn round
      ([x] (round :normal x))
      ([mode x]

        (let [fn (case mode
                   :high    (static-fn Math/ceil)
                   :low     (static-fn Math/floor)
                   :normal  (static-fn Math/round)
                   (throw (Exception.
                           "ERROR: round [:high|:low|:normal] <VALUE>")))]
          (long (fn x)))))

    (defn get-percentage
      ([place total-count] (get-percentage :normal place total-count))
      ([mode place total-count]
        (if-not (contains? #{:high :low :normal} mode)
                (throw (Exception.
                        "ERROR: get-percentage [:high|:low|:normal] <PLACE> <TOTAL_COUNT>")))
        (round mode (/ (* place 100.0) total-count))))
​

​I made two other handy functions:
    (defn round-precision [value precision]
      (let [multiplier (Math/pow 10.0 precision)]
        (/ (Math/round (* value multiplier)) multiplier)))
   
   
    (defn round-div-precision [dividend divisor precision]
        (round-precision (/ (float dividend) divisor) precision))

For example:
    (round-div-precision 22000 7 3)

gives:
    3142.857

​

and:
    (round-div-precision 22000 7 -3)

gives:
    3000.0

--

Cecil Westerhof

[Atamert Ölçgen]

On Sat, Feb 14, 2015 at 6:08 PM, Cecil Westerhof <cldwes...@gmail.com> wrote:

2015-02-14 12:33 GMT+01:00 Cecil Westerhof <cldwes...@gmail.com>:

2015-02-11 8:32 GMT+01:00 Cecil Westerhof <cldwes...@gmail.com>:

I needed a function to get the percentage as an int. Input is place and total-count.
I want the normal definition (which is the default) and a high and low variant.

I came up with the following code:
    (defn get-percentage
      ([place total-count] (get-percentage :normal place total-count))
      ([mode place total-count]
        (let [percentage

​​

(/ (* place 100.0) total-count)]
          (condp = mode
            :high     (int (Math/ceil  percentage))
            :low      (int (Math/floor percentage))
            :normal   (int (Math/round percentage))

I don't understand why you prefer this over Phill's suggestion (using comp and having 3 versions of get-percentage). Then you wouldn't need the extra arity (2 param one) too.

Phill's version is 7 lines of code, 4 simple defn's & 3 def's. I'm just saying this in case it was overlooked.

 

--

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
---
You received this message because you are subscribed to the Google Groups "Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Kind Regards,
Atamert Ölçgen

-+-
--+
+++

www.muhuk.com

[Cecil Westerhof]

2015-02-15 10:23 GMT+01:00 Atamert Ölçgen <mu...@muhuk.com>:

On Sat, Feb 14, 2015 at 6:08 PM, Cecil Westerhof <cldwes...@gmail.com> wrote:

2015-02-14 12:33 GMT+01:00 Cecil Westerhof <cldwes...@gmail.com>:

2015-02-11 8:32 GMT+01:00 Cecil Westerhof <cldwes...@gmail.com>:

I needed a function to get the percentage as an int. Input is place and total-count.
I want the normal definition (which is the default) and a high and low variant.

I came up with the following code:
    (defn get-percentage
      ([place total-count] (get-percentage :normal place total-count))
      ([mode place total-count]
        (let [percentage

​​

(/ (* place 100.0) total-count)]
          (condp = mode
            :high     (int (Math/ceil  percentage))
            :low      (int (Math/floor percentage))
            :normal   (int (Math/round percentage))

I don't understand why you prefer this over Phill's suggestion (using comp and having 3 versions of get-percentage). Then you wouldn't need the extra arity (2 param one) too.

Phill's version is 7 lines of code, 4 simple defn's & 3 def's. I'm just saying this in case it was overlooked.

​One reason is that now I can use a parameter to decide which version to call. If there is a part of code where the version of get-percentage is depending on a parameter, I just can use:

    (get-percentage mode place total-count)

Instead of having to use a condition block at that place.

--

Cecil Westerhof