Implementing rail-fence-cipher #627

Average-user · 2017-11-29T01:23:17Z

I'm not sure about the types on src/RailFenceCipher.hs

Should I change them to f :: Int -> String -> String or change the ones from the tests?

Average-user · 2017-11-29T11:33:35Z

Or maybe change the example to:

encode' :: (Ord a) => Int -> [a] -> [a]
encode' n xs = concat [[a | (a, b) <- zip xs c, b == i] | i <- [1..n]]
  where c = cycle $ [1..n] ++ [n-1,n-2..2]

encode :: Int -> String -> String
encode = encode'

decode :: Int -> String -> String
decode n xs = map snd $ sort $ zip (encode' n [0..length xs - 1]) xs

petertseng

seems good, seems good. I think I have an answer to your question, and a question to ask.

petertseng · 2017-11-30T08:38:38Z

exercises/rail-fence-cipher/examples/success-standard/src/RailFenceCipher.hs

+
+import Data.List (sort)
+
+encode :: (Ord a) => Int -> [a] -> [a]


Will you explain to me why (Ord a) => necessary? I am not seeing why, but I may have just missed it.

Seems like it isn't, I think it was from a previous version

Wow my bad there, seems like in the sort is needed, cause it takes on count the element of the pairs

Should I leave it as it is? Or use sortBy?

to sum up, I think that there are 2 options:

decode :: Int -> [a] -> [a] decode n xs = map snd $ sortBy (compare `on` fst) zippedL where zippedL = zip (encode n [0..length xs - 1]) xs

or what we already have.

I see, because you are using sort in decode.

In contrast to the stub, where I argued that we only need show the student what was specified by the problem description, here in the example I would in fact prefer not to add constraint that isn't necessary. Because the example is doing well by expanding from String to some other type. But if it's stopping at (Ord a) then it's only going halfway! It should go all the way!

Thus, let us use sortBy here!

petertseng · 2017-11-30T08:38:45Z

exercises/rail-fence-cipher/examples/success-standard/src/RailFenceCipher.hs

+encode n xs = concat [[a | (a, b) <- zip xs c, b == i] | i <- [1..n]]
+  where c = cycle $ [1..n] ++ [n-1,n-2..2]
+
+decode :: (Ord a) => Int -> [a] -> [a]


Will you explain to me why (Ord a) => necessary? I am not seeing why, but I may have just missed it.

same question

same answer

petertseng · 2017-11-30T08:39:50Z

exercises/rail-fence-cipher/src/RailFenceCipher.hs

@@ -0,0 +1,7 @@
+module RailFenceCipher (encode, decode) where
+
+encode :: Int -> [a] -> [a]


I see you ask whether to change this. I would use String here simply because that is all tests will use, and it's fine if students realise it can be generalised and therefore change it back to [a]. I would therefore also not change the example, minus the comment I make on it.

But why make it less general when it works fine with other types?

One thing I see is that https://github.com/exercism/problem-specifications/blob/master/exercises/rail-fence-cipher/description.md only talks about working with strings, thus that is the extent of the problem being specified. Therefore, any function that I write that adheres to this specification need only operate with strings. It is of course great if it works with other types. I imagine most solutions will. The possible complaint with putting [a] here is that "why is this stub asking for more than the problem specification is asking for?" the possible complaint with putting String is "this stub is unnecessarily restricting this function to only work with the specification, but it could do more"

It's not a perfect analogue, but there are text like https://github.com/exercism/haskell/blob/master/exercises/leap/HINTS.md or https://github.com/exercism/haskell/blob/master/exercises/series/HINTS.md that we can add.

I don't remember right now whether this track has a policy or other examples of either choice ("give minimal signature that satisfies the specification and let students expand" or "give the most general signature that a reasonable solution should be able to achieve") and I can't go looking for examples now, so I make my recommendation based on what I can see now, but I would change if there is an example of something we have done in the past.

Well, I thing with Int -> String -> String would be just fine, afterall

petertseng · 2017-11-30T08:40:00Z

exercises/rail-fence-cipher/src/RailFenceCipher.hs

+encode :: Int -> [a] -> [a]
+encode = "You need to implement this function!"
+
+decode :: Int -> [a] -> [a]


I see you ask whether to change this. I would use String here simply because that is all tests will use, and it's fine if students realise it can be generalised and therefore change it back to [a]. I would therefore also not change the example, minus the comment I make on it.

same comment

Ahh ok, dint read this before

petertseng · 2017-11-30T08:40:13Z

exercises/rail-fence-cipher/README.md

@@ -0,0 +1 @@
+## pls regenerate this!


I remind myself to regenerate.

petertseng · 2017-11-30T08:43:54Z

exercises/rail-fence-cipher/src/RailFenceCipher.hs

+module RailFenceCipher (encode, decode) where
+
+encode :: Int -> [a] -> [a]
+encode = "You need to implement this function!"


don't forget error!

petertseng · 2017-11-30T08:44:04Z

exercises/rail-fence-cipher/src/RailFenceCipher.hs

+encode = "You need to implement this function!"
+
+decode :: Int -> [a] -> [a]
+decode = "You need to implement this function!"


don't forget error!

same

whoops!

same :(

petertseng

let's do it.

Average-user · 2017-12-04T13:30:08Z

let's do it.

Do what?

Average-user · 2017-12-04T13:30:25Z

ohh, I see.

petertseng · 2018-01-06T06:00:33Z

exercises/rail-fence-cipher/package.yaml

@@ -0,0 +1,20 @@
+name: rail-fence-cipher
+version: 1.1.0.1


my mistake. it was 1.0.1 not 1.1.0. Fixed in #634

Implementing rail-fence-cipher

59f30ea

petertseng suggested changes Nov 30, 2017

View reviewed changes

petertseng reviewed Nov 30, 2017

View reviewed changes

Average-user and others added 2 commits November 30, 2017 16:24

small changes on examples and src

0af733a

rail-fence-cipher: gen readme

268b45a

petertseng approved these changes Dec 4, 2017

View reviewed changes

petertseng merged commit c0f56ed into exercism:master Dec 4, 2017

petertseng reviewed Jan 6, 2018

View reviewed changes


		import Data.List (sort)

		encode :: (Ord a) => Int -> [a] -> [a]

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

		encode :: Int -> [a] -> [a]

Uh oh!

Implementing rail-fence-cipher #627

Implementing rail-fence-cipher #627

Uh oh!

Conversation

Average-user commented Nov 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Average-user commented Nov 29, 2017

Uh oh!

petertseng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Average-user Nov 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petertseng Nov 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petertseng left a comment

Choose a reason for hiding this comment

Uh oh!

Average-user commented Dec 4, 2017

Uh oh!

Average-user commented Dec 4, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Average-user commented Nov 29, 2017 •

edited

Loading

Average-user Nov 30, 2017 •

edited

Loading

petertseng Nov 30, 2017 •

edited

Loading