Fork me on GitHub
#code-reviews
<
2019-10-21
>
Chase16:10:07

I would love to get some feedback on my run length encoding solution from exercism if possible. It passes all tests but doesn't seem very elegant. I feel like a lot of the "hackiness" comes from dealing with the edge case where you only have one char. https://pastebin.com/4ZFw5dQC

Chase16:10:21

I also find myself wanting to put detailed expected shape of input data and output data to make it clearer but when trying to put examples in the doc strings I'm getting a lot of push back from the repl. How do you folks deal with that?

Chase16:10:38

also not sure if there is a way to link to the generic exercism page showing the problem statement if that helps with advice.

dominicm16:10:46

Why do you find that hard in the repl?

Chase16:10:46

when I try to put stuff like `Example input (\A \A) -> output "2A") it yells at the use of the back slash and quotes.

Chase16:10:03

so then I'm trying to throw in more back slashes as escape thingies but it turns into an unreadable mess

dominicm16:10:48

Ah, so more about clojure's string rules than the repl?

dominicm16:10:56

Are you using editor integration?

Chase16:10:05

yeah. I would like if anything in the doc string is just ignored but it seems when I evaluate the function in my repl if I've added doc strings like the example above it no longer works.

dominicm16:10:26

There's some tickets around for heredoc, but I think they're unlikely to happen. I've never found it so much of a problem though. Characters are an unfortunate edge case.

Chase16:10:04

ok cool. So one thing I've found with reading clojure (including stuff I wrote a week ago) is not understanding what the expected data looks like. Maybe I'm making too big a deal out of that though.

dominicm16:10:59

No. I think docs are important.

dominicm16:10:04

Don't get me wrong.

dominicm16:10:13

I try to write that stuff down.

Chase16:10:55

ok. good. because looking at my functions there can you tell what they even do?

Chase16:10:08

naming was hard. haha

dominicm16:10:06

Vaguely. It was a little tough though.

Chase16:10:37

lol, right?! That's how I feel reading everyone's clojure code. Maybe that's how I feel reading any code though, I don't know

dominicm16:10:04

When I read library code, not so much. When I read application code, all the time.

dominicm16:10:17

I'd prefer application code didn't feel that way though

Chase16:10:27

at least my confidence is growing that I'll be able to hack up a solution to a problem now. I'm just not so sure I'll understand my own solution after a little time has passed. baby steps I guess

dominicm16:10:01

You get better at reading the implementation over time to predict.

Chase16:10:47

I keep thinking if I could just see the shape of expected data for input and output I would be in a good place. Haven't figured out the best way to solve that desire. I guess if I find some code in the wild I could try and get it in the repl and play around with it.

Chase16:10:13

Anyways, thank you for the chat! I'm still open on suggestions for how to improve this solution too

dominicm16:10:11

The =1 stuff is a bit suspect, you're right :)

Chase16:10:20

yeah, it doesn't sit right with me. The thing is input can be like "AAABCCC" but the output can't be "3A1B3C"

jaihindhreddy08:10:38

Your code fails if something appears more than 9 times. This would require the length to be 2 digits, and you're only ever considering one. You can use the fact that mapcat treats nils as empty seqs to improve the special case a bit, and also, regex groups to help you separate the count from the element instead of using last and butlast eliminating the bug I mentioned above. Some destructuring later, here's what I ended up with:

(defn encode [txt]
  (->> (partition-by identity txt)
       (mapcat #(let [[n x] ((juxt count first) %)]
                  [(when (> n 1) n) x]))
       (apply str)))

(defn decode [encoded]
  (->> (re-seq #"(\d+)?([a-zA-Z\s])" encoded)
       (mapcat (fn [[_ n x]]
                 (repeat (Long/parseUnsignedLong (or n "1")) x)))
       (apply str)))
Sorry if irrelevant or too late now 😀

jaihindhreddy08:10:05

The juxt might be going too overboard and into golfing land. You could do:

(defn encode [txt]
  (->> (partition-by identity txt)
       (mapcat #(let [n (count %)
                      x (first %)]
                  [(when (> n 1) n) x]))
       (apply str)))

Chase09:10:06

thanks for the advice! I got some feedback from the exercism mentor too so will be doing some refactoring with both your suggestions in mind.

Chase09:10:08

My solution does work for occurrences more than 9 times though. I remember having to go back and tweak it when getting that failed test. But now it passes. I think that's what the + after my \d regex covers.

👍 4
jaihindhreddy09:10:54

You're right, I read that wrong.