2023-09-28 code-reviews | Clojure Slack Archive

code-reviews

roelof 2023-09-28T13:36:12.325629Z

What can I improve on this code :

(ns interest-is-interesting)

(defn interest-rate
  "Returns the interest rate based on the specified balance."
  [balance]
  (cond 
    (< balance 0 ) -3.213
    (< balance 1000) 0.5
    (< balance 5000) 1.621
    :else 2.475 ))

(defn balance-update-amount 
  "Returns the amount which updates the balance."
  [balance]
  (let [interest-rate (bigdec (/ (interest-rate balance) 100))]
  (* (abs balance) interest-rate)))
   

(defn annual-balance-update
  "Returns the annual balance update, taking into account the interest rate."
  [balance]
  (+ balance (balance-update-amount balance))) 

(defn amount-to-donate
  "Returns how much money to donate based on the balance and the tax-free percentage."
  [balance tax-free-percentage]
  (if (> balance 0)
  (int (* (* balance (/ tax-free-percentage 100)) 2 ))
    0))

Rupert (Sevva/All Street) 2023-09-28T19:29:48.495939Z

Generally looks pretty good. • If you are going to use big decimal then use it everywhere and be very careful its not being converted back into a regular double along the way. ◦ bigdecimal is probably overkill compared to double - but it's subjective and some interviewers will prefer it. ◦ (type (balance-update-amount 8.0)) => double which means the conversion to big decimal didn't really help. • Do you really mean to convert amount into an integer - perfer (long x) over (int x) and you probably want (float x) anyway. • You can use condp instead of cond • This is subjective, but I prefer making my conditional statements as small as possible. ◦ (long (* (* (if (> balance 0) balance 0) (/ tax-free-percentage 100)) 2 )) So only minor nitpicks really.

roelof 2023-09-28T19:31:03.057429Z

Thanks

roelof 2023-09-28T19:31:25.244379Z

The tests want that some outcomes are big decimals

Rupert (Sevva/All Street) 2023-09-28T19:31:38.802719Z

You can also unest your (* (* => (*

roelof 2023-09-28T19:31:40.498249Z

That is why I convert it to a big dec

roelof 2023-09-28T19:31:57.340779Z

and the last one needed to be a int

Rupert (Sevva/All Street) 2023-09-28T19:32:06.656899Z

Yes- but you're converting back in the next line by using (*

Rupert (Sevva/All Street) 2023-09-28T19:32:36.656089Z

So you should then use big double methods for multiplication / addition etc to keep the output as bigdouble too.

Rupert (Sevva/All Street) 2023-09-28T19:32:54.554529Z

Unnesting gives: (* (if (> balance 0) balance 0) (/ tax-free-percentage 100) 2 )

roelof 2023-09-28T19:33:06.871299Z

What is then the best way to keep it a bigdec ?

Rupert (Sevva/All Street) 2023-09-28T19:33:40.884789Z

You can use the methods on the object: https://docs.oracle.com/javase/8/docs/api/java/math/BigDecimal.html

roelof 2023-09-28T19:35:07.851839Z

oke, I will read that page

roelof 2023-09-28T19:38:36.453179Z

Thanks a lot for your feedback

Rupert (Sevva/All Street) 2023-09-28T19:38:47.819279Z

No probs!

Clojurians Log v2

code-reviews