HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Number of Chocolates to Gluttonize in Clojure

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numberclojuregluttonizechocolates

Problem

I’ve been dabbling in Clojure by solving programming puzzles. This particular puzzle is from HackerRank, with the objective of figuring out how much chocolate one could eat given three parameters:

  • The cost of a piece of chocolate



  • How much money you have



  • The candy shop owner will give you a free chocolate for x number of wrappers



In the code below, those three numbers are passed to total-chocolates-consumed which returns the answer.

(ns hr.clojure.algorithms.implementation.implementation.chocolate-feast)

(defn int-division
  [numerator denominator]
  (->>
    (/ numerator denominator)
    int))

(defn number-to-purchase
  [totalToSpend costPerChocolate]
  (int-division
    totalToSpend
    costPerChocolate))

(defn number-of-freebies
  [totalWrappers wrappersPerFreebie]
  [(int-division
    totalWrappers
    wrappersPerFreebie)
   (int
     (mod
       totalWrappers
       wrappersPerFreebie))])

(defn total-chocolates-consumed
  [totalToSpend
   costPerChocolate
   wrappersPerFreebie]
   (let [purchasedChocolates (number-to-purchase totalToSpend costPerChocolate)
         freebies (first (number-of-freebies purchasedChocolates wrappersPerFreebie))
         totalChocolates [purchasedChocolates freebies]
         startRemainder (second (number-of-freebies purchasedChocolates wrappersPerFreebie))]
         (loop
           [chocs totalChocolates
            remainder startRemainder]
           (cond
            (> wrappersPerFreebie (+ remainder (last chocs)))
               (reduce + chocs)
             :else
              (recur
                (conj chocs (first (number-of-freebies (+ (last chocs) remainder) wrappersPerFreebie)))
                (second (number-of-freebies (+ (last chocs) remainder) wrappersPerFreebie)))))))


I've written tests for this code here:

```
(ns hr.clojure.algorithms.implementation.implementation.chocolate-feast.spec)

(load-file "YOUR-PATH-TO-FILE")
(use 'hr.clojure.algorithms.implementation.implementation.chocol

Solution

General Questions

  • Yes. Your tests are fine.



  • I found the code hard to follow without the requirements. To some degree, that's to be expected. Even once I read the requirements, there were things that I found hard to follow. More on that later.



How idiomatic is my code?

  • Never use camel case for your clojure-defined symbols.



  • You should always use lower-cased, hyphenated names in Clojure.



-
Your formatting was fine, except the let statement. It should be formatted like so:

(let [my-thing :a
      other-thing :b]
  (code-that-does-stuff ...))


The clojure style guide should tell you everything you need to know about styling your code.

What is a good way to communicate what a function does?

This is a complex question. My first piece of advice is to accurately name the function. number-of-freebies implies a number, but you are returning a vector. So a more appropriate name might be division-tuple or, better, quotient-remainder-tuple.

Regarding return types, the options you have, in order of increasing ceremony, are:

  • Return a tuple.



  • Return a map whose keys denote the return values.



  • Define a type or record.



For this example, I think a tuple is fine. If I thought the return value might need to expand in the future, I'd consider using a map. I generally use records and types when:

  • I'm designing a large(er) system (i.e. doing design work as opposed to implementing functions)



  • I want to reify a complex type



  • I want type dispatch



So, for something like this, I'd consider types to be a little overkill. Your tastes may vary.

How do I refactor total-chocolates-consumed to be less complex?

I implemented a solution in preparing your feedback, and the most straightforward method I could come up with on the fly was indeed recursive. Though, I will say this tends to be the kind of problem that you realize can be solved with map/reduce after a night or two of sleep.

There are two things you can do to make total-chocolates-consumed less complex:

-
Use destructuring:

(let [[freebies startRemainder] (number-of-freebies purchasedChocolates wrappersPerFreebie)]
  ...)


-
Use more let statements. Specifically inside the loop-recur.

I have a version of total-chocolates-consumed that does those things if you want to see it. I thought you might like to take a stab at it before you see what I did.

Other Notes

When I moved from Java to Clojure, I tended to like very small functions (e.g. number-to-purchase or int-division).
Over time, I've found it's a) more idiomatic and b) easier to read if you start with a rather large function and break it up as needed to increase understanding.
My reasoning for this preference is that Clojure already gives you really good, simple building blocks.
This allows you to define the simple functions that directly enrich your domain without worrying as much about length.
If I want added documentation, I will let a variable with a good name.

On the topic of int-division, I'm pretty sure you can solve this problem with just quot and rem. Wasn't sure you were aware of those functions.
Feel free to let me know if I'm mistaken there.

Lastly, in my solution, I tried to focus first on calculating the purchased chocolates, then on calculating the freebies,
as opposed to creating a seq of all chocolates, then summing them up at the end. This separation of concerns allows you to structure the program
intent clearly at the topmost level. It has the added benefit of making the recursive part of the program a bit more straightforward.

Update

Per OP's request, here is my solution:

(defn calculate-freebies [purchased-chocolates wrappers-per-freebie]
  (loop [remaining-wrappers purchased-chocolates
         free-chocolates 0]
    (if (< remaining-wrappers wrappers-per-freebie)
      free-chocolates
      (let [new-chocolates (quot remaining-wrappers
                                 wrappers-per-freebie)]
        (recur
          (+ (rem remaining-wrappers
                  wrappers-per-freebie)
             new-chocolates)
          (+ free-chocolates new-chocolates))))))

(defn calculate-chocos [bobs-money cost-of-chocolate wrappers-per-freebie]
  (let [purchased-chocolates (quot bobs-money
                                   cost-of-chocolate)
        freebies (calculate-freebies purchased-chocolates
                                     wrappers-per-freebie)]
    (+ purchased-chocolates freebies)))


Note that I did go ahead and pull out calculate-freebies into a separate defn. I did that because a) it's a standalone concept and b) it makes calculate-chocos easier to read. If it weren't a standalone concept, I would have likely defined it in the let of calculate-chocos itself.

Code Snippets

(let [my-thing :a
      other-thing :b]
  (code-that-does-stuff ...))
(let [[freebies startRemainder] (number-of-freebies purchasedChocolates wrappersPerFreebie)]
  ...)
(defn calculate-freebies [purchased-chocolates wrappers-per-freebie]
  (loop [remaining-wrappers purchased-chocolates
         free-chocolates 0]
    (if (< remaining-wrappers wrappers-per-freebie)
      free-chocolates
      (let [new-chocolates (quot remaining-wrappers
                                 wrappers-per-freebie)]
        (recur
          (+ (rem remaining-wrappers
                  wrappers-per-freebie)
             new-chocolates)
          (+ free-chocolates new-chocolates))))))

(defn calculate-chocos [bobs-money cost-of-chocolate wrappers-per-freebie]
  (let [purchased-chocolates (quot bobs-money
                                   cost-of-chocolate)
        freebies (calculate-freebies purchased-chocolates
                                     wrappers-per-freebie)]
    (+ purchased-chocolates freebies)))

Context

StackExchange Code Review Q#98817, answer score: 8

Revisions (0)

No revisions yet.