Add AOC Day 03 solution #5

shahn95 · 2023-05-12T08:49:59Z

AoC Day 03 ready for review

advent-of-code/rucksack-problem/.calva/output-window/output.calva-repl

shhivam · 2023-05-18T08:38:06Z

advent-of-code/rucksack-problem/.gitignore

+.hgignore
+.hg/


Are we using mercurial for code versioning anywhere?

These were auto-generated, haven't looked into this yet.

advent-of-code/rucksack-problem/src/rucksack_problem/core.clj

tfidfwastaken

Good:

Stateless pure functions
I/O happens at the boundaries
Threading used to good effect
Overall, nice solution!

Comments about what to improve are below.

tfidfwastaken · 2023-05-25T06:38:43Z

advent-of-code/rucksack-problem/CHANGELOG.md

@@ -0,0 +1,24 @@
+# Change Log


Delete this file?

Done, file deleted.

tfidfwastaken · 2023-05-25T06:38:58Z

advent-of-code/rucksack-problem/README.md

@@ -0,0 +1,44 @@
+# rucksack-problem


Fill in some short instructions here, or remove them if not relevant.

Yes, I added a brief description.

tfidfwastaken · 2023-05-25T06:39:05Z

advent-of-code/rucksack-problem/project.clj

@@ -0,0 +1,10 @@
+(defproject rucksack-problem "0.1.0-SNAPSHOT"
+  :description "FIXME: write description"


likewise, here. Address the FIXMEs

Filled in the FIXMEs

tfidfwastaken · 2023-05-25T06:39:14Z

advent-of-code/rucksack-problem/src/rucksack_problem/core.clj

+;; 2. find priority of common item
+;; Repeat the above for each group of 3 rucksacks, and sum the priorities of common items.
+
+(ns rucksack-problem.core


In general, try to avoid calling your namespace core. It doesn't say anything about what this module does.

In this case, it's not a big deal, since we are having only a single module, but this is something to keep in mind when you move to multi-namespace projects.

Good point, I've been going with the default core. Would solution be a good name for this namespace?

tfidfwastaken · 2023-05-25T06:39:18Z

advent-of-code/rucksack-problem/src/rucksack_problem/core.clj

+(defn compartmentalize
+  "Divide rucksack contents into 2 compartments
+   1 string -> 2 lists of chars"
+  [rucksack-contents]
+  (let [half-length (/ (count rucksack-contents) 2)]
+    (->> rucksack-contents
+         (partition half-length))))


[suggestion] Consider a more precise name, like partition-half.

A good heuristic for checking if the name is good: Can you guess what this function does if you were only given its name (and no access to the docstring or implementation)?

Applying this heuristic here:

compartmentalize tells us that I can give it a rucksack and I get something that arranges its items into compartments. But it doesn't tell us how many compartments and criteria for which it makes them.

Agreed, partition-half is very clear.
I've been having trouble deciding when to use words from the problem set (compartmentalize) vs. words that more clearly describe what the function does. Will now tend towards the latter, that is, more descriptive function names.

tfidfwastaken · 2023-05-25T06:39:31Z

advent-of-code/rucksack-problem/src/rucksack_problem/core.clj

+  (let [c1 (set compartment-1)
+        c2 (set compartment-2)]
+    (some #(get c2 %) c1)))
+;; can also use set intersection here


Why not use a set intersection, then? It's a well-known operation, and you are using sets here after all.

tfidfwastaken · 2023-05-25T06:39:33Z

advent-of-code/rucksack-problem/src/rucksack_problem/core.clj

+(defn find-common-item
+  "Find the common item between 2 compartments"
+  [compartment-1, compartment-2]
+  (let [c1 (set compartment-1)
+        c2 (set compartment-2)]
+    (some #(get c2 %) c1)))
+;; can also use set intersection here
+
+(defn find-badge
+  "Find the common item (badge) between a group of 3 rucksacks.
+   Used in Part 2 of problem."
+  [rucksack-1, rucksack-2, rucksack-3]
+  (let [r1 (set rucksack-1)
+        r2 (set rucksack-2)
+        r3 (set rucksack-3)]
+    (first (set/intersection r1 r2 r3))))


These two functions have some duplicated logic. They are both finding a common item in sequences of items. Consider having these delegate to a generic utility function.

Yes, the logic is duplicated.
So find-common-item and find-badge will remain, and they would both call a new utility function within themselves. I'll give this a go.

The new utility function will take minimum 2 + any number of collections as a rest argument. It will convert these to sets, and then return the first common element between the collections. (We only expect one common item as per the problem statement).

tfidfwastaken · 2023-05-25T06:39:35Z

advent-of-code/rucksack-problem/src/rucksack_problem/core.clj

+  (let [m1 (frequencies compartment-1)
+        m2 (frequencies compartment-2)]
+    (reduce
+     (fn [m [item frequency]]
+       (if (contains? m2 item)
+         (->> item
+              (get m2)
+              (min frequency)
+              (assoc m item))
+         m))
+     {} m1)))


Some thoughts regarding this bit (I believe I shared some of these from our pairing session, repeating them here):

Try to refactor this without using reduce. Reach for clojure.core utilities.

If it can't be done with clojure core, then try it with reduce.

Another source of complexity in this reduce is the dependence on state in the enclosing function, ie, m2. You could clump it in the accumulator, which would let you extract the reducing function. It would still not be easy to read, though.

Consider more descriptive names, m, m1, m2 make this harder to reason about. Off the top of my head:

m -> common-item-frequency

m1 -> item-freq-c1, m2 -> item-freq-c2

Sometimes reduce is not a good fit because of readability, in which case use loop-recur as a last resort.

I'd encourage you to actually write this function in all the ways discussed above and compare them.

tfidfwastaken · 2023-05-25T06:39:40Z

advent-of-code/rucksack-problem/src/rucksack_problem/core.clj

+  (println "Part 2 answer: \n"
+           (->> "input.txt"
+                input-file->string-seq
+                sum-badge-priorities)))


[style nitpick] Add a newline at the end of the files.

Yes, will do. Looked it up as well, didn't know about this!

Add AOC Day 03 solution

dd2f849

shahn95 requested review from yudistrange, prabhanshuguptagit, tfidfwastaken, shhivam, PriyangaPKini and qptr May 12, 2023 08:50

shahn95 marked this pull request as draft May 18, 2023 08:34

shhivam reviewed May 18, 2023

View reviewed changes

advent-of-code/rucksack-problem/.calva/output-window/output.calva-repl Outdated Show resolved Hide resolved

shhivam reviewed May 18, 2023

View reviewed changes

advent-of-code/rucksack-problem/src/rucksack_problem/core.clj Outdated Show resolved Hide resolved

shahn95 added 8 commits May 18, 2023 18:02

Refactor functions to improve readability and simplicity

fece201

Add AoC day 03 part 02 solution

7e4548e

Require set namespace

eb9481a

Add tests for AoC Day03 Part 02

0e79797

Refactor compartmentalize and sum-rucksack-priorities tests

e889c47

Reorder tests for AoC Day 03

73dc735

Basic reformatting for AoC Day 03 core.clj file

ee02cf7

Remove output.calva-repl file and stop tracking .calva folder

67b3b8a

shahn95 marked this pull request as ready for review May 23, 2023 17:09

tfidfwastaken requested changes May 25, 2023

View reviewed changes

shahn95 removed request for prabhanshuguptagit, PriyangaPKini, qptr and yudistrange June 5, 2023 07:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add AOC Day 03 solution #5

Add AOC Day 03 solution #5

shahn95 commented May 12, 2023 •

edited

Loading

shhivam May 18, 2023

shahn95 May 18, 2023

tfidfwastaken left a comment

tfidfwastaken May 25, 2023 •

edited

Loading

shahn95 May 29, 2023

tfidfwastaken May 25, 2023

shahn95 May 29, 2023

tfidfwastaken May 25, 2023

shahn95 May 29, 2023

tfidfwastaken May 25, 2023

shahn95 May 29, 2023

tfidfwastaken May 25, 2023

shahn95 May 29, 2023 •

edited

Loading

tfidfwastaken May 25, 2023

tfidfwastaken May 25, 2023

shahn95 May 29, 2023

shahn95 May 29, 2023

tfidfwastaken May 25, 2023 •

edited

Loading

tfidfwastaken May 25, 2023

shahn95 May 29, 2023

		@@ -0,0 +1,10 @@
		(defproject rucksack-problem "0.1.0-SNAPSHOT"
		:description "FIXME: write description"

		.hgignore
		.hg/

Add AOC Day 03 solution #5

Are you sure you want to change the base?

Add AOC Day 03 solution #5

Conversation

shahn95 commented May 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tfidfwastaken left a comment

Choose a reason for hiding this comment

tfidfwastaken May 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shahn95 May 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tfidfwastaken May 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shahn95 commented May 12, 2023 •

edited

Loading

tfidfwastaken May 25, 2023 •

edited

Loading

shahn95 May 29, 2023 •

edited

Loading

tfidfwastaken May 25, 2023 •

edited

Loading