Backport HW1 discussion.
This commit is contained in:
parent
2a4d492810
commit
6dedc07fd3
83
HW1.md
Normal file
83
HW1.md
Normal file
|
@ -0,0 +1,83 @@
|
||||||
|
**Andrew\'s solution**:
|
||||||
|
|
||||||
|
(Hi Andrew! Long time since databases)
|
||||||
|
|
||||||
|
This solution is straightforward and simple, and is similar to
|
||||||
|
Nicholas\'. Just like all the other solutions, it pattern matches on the
|
||||||
|
arguments (instead of using \"case\" expressions), and uses the Prelude
|
||||||
|
functions. Everything\'s idiomatic, but I think that the last function
|
||||||
|
(\"pretty\") can use an improvement. Here\'s the code in question:
|
||||||
|
|
||||||
|
pretty (Mul l r) = a ++ " * " ++ b
|
||||||
|
where
|
||||||
|
a = case l of
|
||||||
|
(Add _ _) -> parens True (pretty l)
|
||||||
|
_ -> parens False (pretty l)
|
||||||
|
b = case r of
|
||||||
|
(Add _ _) -> parens True (pretty r)
|
||||||
|
_ -> parens False (pretty r)
|
||||||
|
|
||||||
|
Here, the code for \"a\" and \"b\" is almost identical, and uses an
|
||||||
|
unneeded auxiliary function. For one thing, you can merge the two
|
||||||
|
variables \"a\" and \"b\" into a function:
|
||||||
|
|
||||||
|
pretty (Mul l r) = wrap l ++ " * " ++ wrap r
|
||||||
|
where
|
||||||
|
wrap e = case e of
|
||||||
|
(Add _ _) -> parens True (pretty e)
|
||||||
|
_ -> parens False (pretty e)
|
||||||
|
|
||||||
|
Then, you can fuse the definition of \"parens\" with the definition of
|
||||||
|
\"wrap\" (passing booleans to functions is actually a code smell, see
|
||||||
|
[the boolean identity
|
||||||
|
crisis](https://programming-elm.com/blog/2019-05-20-solving-the-boolean-identity-crisis-part-1/)):
|
||||||
|
|
||||||
|
pretty (Mul l r) = wrap l ++ " * " ++ wrap r
|
||||||
|
where
|
||||||
|
parens e@(Add _ _) = "(" ++ pretty e ++ ")"
|
||||||
|
parens e = pretty e
|
||||||
|
|
||||||
|
**Nicholas\' solution**
|
||||||
|
|
||||||
|
This is another straightforward and idiomatic solution. The \"pretty\"
|
||||||
|
function is conservative, unlike Andrew\'s, but requires little code.
|
||||||
|
|
||||||
|
**Shu-Kan\'s solution**
|
||||||
|
|
||||||
|
The parenthesis placement is a little off in this one, and it doesn\'t
|
||||||
|
use built-in functions like \'max\', which could have been used in
|
||||||
|
maxLit. It also seems to import data.Typeable, which I don\'t really see
|
||||||
|
used here. To improve it, take a look at these lines:
|
||||||
|
|
||||||
|
maxLit (Add(x)(y)) = if maxLit(x) > maxLit(y) then maxLit(x) else maxLit(y)
|
||||||
|
maxLit (Mul(x)(y)) = if maxLit(x) > maxLit(y) then maxLit(x) else maxLit(y)
|
||||||
|
|
||||||
|
In Haskell, function application is written with a space: \"f x\"
|
||||||
|
instead of \"f(x)\". Writing \"maxLit(y)\" is equivalent to writing
|
||||||
|
\"maxLit (y)\". But then, you don\'t even need parentheses!
|
||||||
|
|
||||||
|
maxLit (Add x y) = if maxLit x > maxLit y then maxLit x else maxLit y
|
||||||
|
maxLit (Mul x y) = if maxLit x > maxLit y then maxLit x else maxLit y
|
||||||
|
|
||||||
|
Finally, the \"if x \> y then x else y\" idiom is just the \"max\"
|
||||||
|
function in Haskell\'s Prelude:
|
||||||
|
|
||||||
|
maxLit (Add x y) = max (maxLit x) (maxLit y)
|
||||||
|
maxLit (Mul x y) = max (maxLit x) (maxLit y)
|
||||||
|
|
||||||
|
**My solution**
|
||||||
|
|
||||||
|
I went a weird route. For one, I created a Num instance for Expr, so
|
||||||
|
that I could write expressions as \"3\*2\" instead of \"Mul (Lit 3) (Lit
|
||||||
|
2)). This definition is incomplete; it\'s missing the abs, negate, and
|
||||||
|
signum functions. However, it does make for funny definitions, and
|
||||||
|
handles precedence for me!
|
||||||
|
|
||||||
|
In addition, I wrote a \"fold\" function for the Expr data type, and
|
||||||
|
wrote all functions except for \"pretty\" using it. They ended up pretty
|
||||||
|
short:
|
||||||
|
|
||||||
|
leftLit = fold id const const
|
||||||
|
rightLit = fold id (flip const) (flip const)
|
||||||
|
maxLit = fold id max max
|
||||||
|
eval = fold id (+) (*)
|
Loading…
Reference in New Issue
Block a user