Add explicit args to Exercises - Part 1 #504

lpalma · 2017-03-12T12:30:27Z

Part of #473

Perhaps the next PRs will be smaller, so they can be reviewed easier.

I was thinking about some of the exercises covered in this PR and how much more complex they are in comparison to the first 10 - 15 available exercises (the ones I've completed so far).
Once the student get there, he will have crossed a few Haskell / FP concepts, which is one of the goals of this Track.

Consequently, wouldn't eta-conversion also be part of the concepts he should familiarize with? Therefore, I believe that, starting from a complex enough exercise, keeping the function arguments out would create a better learning experience.

rbasso · 2017-03-12T12:55:32Z

Consequently, wouldn't eta-conversion also be part of the concepts he should familiarize with?
Therefore, I believe that, starting from a complex enough exercise, keeping the function arguments out would create a better learning experience.

Is desirable that, after a few exercises, the student start developing point-free skills. Certainly the people solving the last exercises have a at least a basic competence at eta-reduction.

I thought about the explicit arguments for later exercises more as documentation to help the students identify what each argument is, and maybe also as an additional exposure to idiomatic argument naming, which is usually something people slowly learn reading the function's implementations in Hackage.

Following the same reasoning, we also have the pattern-matching, which isn't possible without changing some explicit arguments, and that is also a must-learn syntax.

I'm unsure about what is the best for the track, so I'll copy the other maintainers to see what they think about it. @exercism/haskell

rbasso

I made a few comments about some naming choices, but avoided repeating then in similar cases. Remember that it is just my opinion.

I'll approve and leave for you to decide if you want to change anything. When you decide it is ready to merge, please tell me.

rbasso · 2017-03-12T12:56:36Z

exercises/atbash-cipher/src/Atbash.hs

@@ -1,7 +1,7 @@
 module Atbash (decode, encode) where

 decode :: String -> String
-decode = error "You need to implement this function."
+decode phrase = error "You need to implement this function."


I would normally go for xs for strings, or maybe cipherText for more something more descriptive. I'm not sure.

Edit: phrase is also OK, of course.

I also prefer following the convention (f or g for functions, xs ys for generic lists, etc.) and I'm happy to change.

The reason I choose phrase here instead, was to match the test suite, believing that it would be easier for the student to understand the test cases.

I followed this same approach for other exercises, trying to match the test suite terms whenever possible, but I'm happy to change them as well.

Please feel free to keep or to change the names you chose.

rbasso · 2017-03-12T12:59:02Z

exercises/binary-search-tree/src/BST.hs


 empty :: BST a
 empty = error "You need to implement this function."

 fromList :: Ord a => [a] -> BST a
-fromList = error "You need to implement this function."
+fromList elems = error "You need to implement this function."


For a list of a completely unknown type, the tradition is to use xs, considering that there is no possible descriptive name.

rbasso · 2017-03-12T12:59:35Z

exercises/binary-search-tree/src/BST.hs


 singleton :: a -> BST a
-singleton = error "You need to implement this function."
+singleton elem = error "You need to implement this function."


Single unknown element usually is x.

rbasso · 2017-03-12T13:00:53Z

exercises/connect/src/Connect.hs

@@ -3,4 +3,4 @@ module Connect (Mark(..), winner) where
 data Mark = Cross | Nought deriving (Eq, Show)

 winner :: [String] -> Maybe Mark
-winner = error "You need to implement this function."
+winner board = error "You need to implement this function."


board is certainly more descriptive than xss. 👍

rbasso · 2017-03-12T13:02:32Z

exercises/crypto-square/src/CryptoSquare.hs

@@ -1,4 +1,4 @@
 module CryptoSquare (encode) where

 encode :: String -> String
-encode = error "You need to implement this function."
+encode input = error "You need to implement this function."


Here, input doesn't add anything. I would normally go with xs, or something more descriptive, like plainText.

rbasso · 2017-03-12T13:04:13Z

exercises/custom-set/src/CustomSet.hs

@@ -19,40 +19,40 @@ import Prelude hiding (null)
 data CustomSet a = Dummy deriving (Eq, Show)

 delete :: a -> CustomSet a -> CustomSet a
-delete = error "You need to implement this function."
+delete elem set = error "You need to implement this function."


I never thought about how to name sets...set seems ok. 👍

rbasso · 2017-03-12T13:04:42Z

exercises/difference-of-squares/src/Squares.hs

@@ -1,10 +1,10 @@
 module Squares (difference, squareOfSums, sumOfSquares) where

 difference :: Integral a => a -> a
-difference = error "You need to implement this function."
+difference n = error "You need to implement this function."


n fits perfectly!

lpalma · 2017-03-12T13:53:22Z

I'm happy with the changes now 👍

rbasso · 2017-03-12T14:01:28Z

Thanks! 😄

Just one final question before merging.

Would you like to squash the commits that affect the same exercise together, to create a more clean history?

lpalma · 2017-03-12T14:05:47Z

Would you like to squash the commits that affect the same exercise together, to create a more clean history?

done 👍

petertseng

Consequently, wouldn't eta-conversion also be part of the concepts

My past opinion at #459 (comment)

summary: I support explicit naming to make clear the meaning of each arg.

I'm certainly happy to be convinced otherwise.

I hope students will not be afraid to eta-reduce when appropriate for the solution.

petertseng · 2017-03-12T22:47:37Z

exercises/crypto-square/src/CryptoSquare.hs

@@ -1,4 +1,4 @@
 module CryptoSquare (encode) where

 encode :: String -> String
-encode = error "You need to implement this function."
+encode xs = error "You need to implement this function."


sorry that I know I'm contradicting #504 (comment) here, but it seemed a bit inconsistent here to use xs but phrase in atbash-cipher?

petertseng · 2017-03-12T22:53:12Z

exercises/largest-series-product/src/Series.hs

@@ -1,4 +1,4 @@
 module Series (largestProduct) where

 largestProduct :: Int -> String -> Maybe Integer
-largestProduct = error "You need to implement this function."
+largestProduct series digits = error "You need to implement this function."


series seems confusing to me since an integer is not really a series. It is the size of the seires we are considering within the digits. I would think either size or span.

lpalma added 3 commits March 11, 2017 18:51

allergies: add explicit args

3a30f88

atbash-cipher: add explicit args

a257914

bank-account: add explicit args

26e373c

rbasso approved these changes Mar 12, 2017

View reviewed changes

lpalma added 13 commits March 12, 2017 14:03

binary-search-tree: add explicit args

799e824

bowling: add explicit args

0d262f2

clock: add explicit args

82828cc

connect: add explicit args

78ea086

crypto-square: add explicit args

460cb35

custom-set: add explicit args

02f03f9

difference-of-squares: add explicit args

10d1906

etl: add explicit args

81242d6

forth: add explicit args

ce484fb

go-counting: add explicit args

7993697

kindergarten-garden: add explicit args

3c288b5

largest-series-product: add explicit args

30cec37

lens-person: add explicit args

3223c18

lpalma force-pushed the add-explicit-args-to-exercises-p1 branch from 5e7c377 to 3223c18 Compare March 12, 2017 14:04

rbasso merged commit 766bb77 into exercism:master Mar 12, 2017

petertseng reviewed Mar 12, 2017

View reviewed changes

lpalma mentioned this pull request Mar 14, 2017

Add Explicit Args To Exercises - Part 2 #506

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add explicit args to Exercises - Part 1 #504

Add explicit args to Exercises - Part 1 #504

lpalma commented Mar 12, 2017

rbasso commented Mar 12, 2017

rbasso left a comment •

edited

Loading

rbasso Mar 12, 2017 •

edited

Loading

lpalma Mar 12, 2017

rbasso Mar 12, 2017

rbasso Mar 12, 2017

rbasso Mar 12, 2017

rbasso Mar 12, 2017

rbasso Mar 12, 2017

rbasso Mar 12, 2017

rbasso Mar 12, 2017

lpalma commented Mar 12, 2017

rbasso commented Mar 12, 2017

lpalma commented Mar 12, 2017

petertseng left a comment

petertseng Mar 12, 2017

rbasso Mar 13, 2017

petertseng Mar 12, 2017

Add explicit args to Exercises - Part 1 #504

Add explicit args to Exercises - Part 1 #504

Conversation

lpalma commented Mar 12, 2017

rbasso commented Mar 12, 2017

rbasso left a comment • edited Loading

Choose a reason for hiding this comment

rbasso Mar 12, 2017 • 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

lpalma commented Mar 12, 2017

rbasso commented Mar 12, 2017

lpalma commented Mar 12, 2017

petertseng left a 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

rbasso left a comment •

edited

Loading

rbasso Mar 12, 2017 •

edited

Loading