Hacker News new | ask | show | jobs
by quchen 4194 days ago
> I wrote this code putting brevity over readability, which is something I usually never do

Shouldn't the point of such a post be to show interesting code? I'm having trouble reading through the densely packed source.

In addition to tromp's minor nitpick, I have several major ones.

- the code is full of redundant parentheses. HLint can detect those (and many other style errors) automatically. LPaste has HLint installed so you have a linting pastebin available online. http://lpaste.net/116871

- A lot of the functions are written in a non-idiomatic way. "m >>= return . f" is "fmap", "(.)" can combine functions much more readable than Lisp stacks of parentheses.

- ByteString.Char8 is usually a wrong choice, more on that here: https://github.com/quchen/articles/blob/master/fbut.md#bytes...

- If you count to "length x" then often there's a more elegant solution that avoids calculating the length altogether. For example "splits xs = zip (inits xs) (tails xs)".

- Brevity is never better than readability.

- No top-level definitions should lack a type signature. GHC even has warnings for that (I think they start firing with -W).

- A function should do one thing and then be composed with other functions. "lowerWords" converts to words and then maps them all to lower case, for example. These are two completely different operations in one long line.

- In order of increasing generality: foldr union empty = unions = mconcat = fold

- Use pattern matching, avoid "(!!)". transposes w = [ a ++ [b0,b1] ++ bs | (a, b0:b1:bs) <- splits w] - also see https://github.com/quchen/articles/blob/master/fbut.md#head-...

- For large amounts of words that you split and concatenate again, String is probably not the right type. Text is good for dealing with such things.

- replaces w = [as ++ [c] ++ bs | (as, _:bs) <- splits w , c <- alphabet]

... and so on.

2 comments

Hi, author here. I think you partially missed the main purpose of the article, which for me was just having fun by playing with a language I'm currently learning. I wasn't try to teach anything to anyone.

But I must say, thanks for the great feedback! Lots of stuff I didn't know that we'll make me write better Haskell code :)

Well, what you wrote is "The main reason I did it was to see what Haskell is capable of compared to other languages such as Python." The problem is that what you coded isn't what Haskell is capable of :)
It would be nice if you explicitly state you are learning Haskell in your article. If I'm a newbie and I see some of the awkward Haskell code(which I also make sometimes), I would feel discouraged. It doesn't seem Haskell is making things better.
I expect there to be lots of comments demonstrating tons of different styles of doing this stuff. Haskell is one of those languages where you can go back to your code 20 times and still find a new way of doing something.
all that, plus, re the question at the end of the article about chooseBest:

    chooseBest ch ws = maximumBy (compare `on` (\w -> M.findWithDefault 0 w ws))
       (S.toList ch)
this is closer to what's in Norvig's article - a single pass over the elements of 'ch', taking the one whose entry in the 'ws' set has the highest count - treating words not in the set as having a count of 0. No sort required. (maximumBy is from Data.List, on is from Data.Function)