<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">I’m no guru, so somebody please feel free to correct my advice here. I’m trying to get a bit more active in reading these mailing lists and giving advice as I believe it’ll help me become better in my own understnading. <div class=""><br class=""></div><div class="">First thing, well done! It works! Nice job for the beginning of your haskell journey :) Ill be focusing on your first and third question. Your indentations seem fine here so far.</div><div class=""><br class=""></div><div class="">There are many different approaches you could take as you learn more, but here are some things I noticed:</div><div class=""><br class=""></div><div class="">******************** 1)</div><div class=""><div style="background-color: rgb(255, 255, 255); line-height: 18px;" class=""><div style="color: rgb(54, 54, 54); font-family: Menlo, Monaco, "Courier New", monospace; white-space: pre;" class=""><span style="color: rgb(99, 99, 36);" class="">isAlphabetic</span> :: <span style="color: rgb(63, 151, 223);" class="">Char</span> -> <span style="color: rgb(63, 151, 223);" class="">Bool</span></div><div class=""><font color="#363636" face="Menlo, Monaco, Courier New, monospace" class=""><span style="white-space: pre;" class="">isAlphabetic char = char >= </span></font><span style="color: rgb(162, 86, 55); font-family: Menlo, Monaco, "Courier New", monospace; white-space: pre;" class="">'A'</span><font color="#363636" face="Menlo, Monaco, Courier New, monospace" class=""><span style="white-space: pre;" class=""> && char <= </span></font><font color="#a25637" face="Menlo, Monaco, Courier New, monospace" class=""><span style="caret-color: rgb(162, 86, 55); white-space: pre;" class="">‘</span></font><span style="color: rgb(162, 86, 55); font-family: Menlo, Monaco, "Courier New", monospace; white-space: pre;" class="">Z'</span></div></div><div><br class=""></div><div>I feel this would be more readable as a literal, though you function works. The <= && => feel very much like imperative style to me</div><div><br class=""></div><div><div style="color: rgb(54, 54, 54); background-color: rgb(255, 255, 255); font-family: Menlo, Monaco, "Courier New", monospace; line-height: 18px; white-space: pre;" class=""><div class=""><span style="color: rgb(99, 99, 36);" class="">isAlphabetic</span> :: <span style="color: rgb(63, 151, 223);" class="">Char</span> -> <span style="color: rgb(63, 151, 223);" class="">Bool</span></div><div class="">isAlphabetic char = elem char [<span style="color: rgb(162, 86, 55);" class="">'A'</span>..<span style="color: rgb(162, 86, 55);" class="">'Z'</span>]</div></div></div><div><br class=""></div><div>That feel nicer to me. ‘Elem' works like ‘in’ in some other languages and has the type: </div><div>elem :: (Eq a, Foldable t) => a -> t a -> Bool</div><div><br class=""></div><div>Lists are a ‘foldable t’ as would be a binary tree (or some other type of tree). Just think of ‘foldable’ as something you can iterate over/through. It as something you can iterate over.</div><div><br class=""></div><div>The ‘..’ Syntax in the expression [‘A’..’Z’] works on any type which is a member of the ‘Enum’ type class, which is to say it has some type of successor/predecessor order. A comes after b, c comes after b and so on. </div><div>So typing [‘A’..’Z’] just creates a literal list of the characters from ‘A’ to ‘Z’.</div><div><br class=""></div><div>Come to think of it, the above would be even nicer if you wrote it in ‘point free style’ (notice the lack of a ‘char’ in my expected isAlphabetic function parameters, despite it being in the type signature) which would require you to ‘flip’ the elem function so the Char becomes the second argument expected rather than the first</div><div><br class=""></div><div><div style="color: rgb(54, 54, 54); background-color: rgb(255, 255, 255); font-family: Menlo, Monaco, "Courier New", monospace; line-height: 18px; white-space: pre;" class=""><div class=""><div style="line-height: 18px;" class=""><div class=""><span style="color: rgb(99, 99, 36);" class="">alphabetLength</span> :: <span style="color: rgb(63, 151, 223);" class="">Int</span></div><div class="">alphabetLength = length alphabet</div><div class=""><br class=""></div></div></div><div class=""><span style="color: rgb(99, 99, 36);" class="">alphabet</span> :: [<span style="color: rgb(63, 151, 223);" class="">Char</span>]</div><div class="">alphabet = [<span style="color: rgb(162, 86, 55);" class="">'A'</span>..<span style="color: rgb(162, 86, 55);" class="">'Z'</span>]</div><br class=""><div class=""><span style="color: rgb(99, 99, 36);" class="">isAlphabetic</span> :: <span style="color: rgb(63, 151, 223);" class="">Char</span> -> <span style="color: rgb(63, 151, 223);" class="">Bool</span></div><div class="">isAlphabetic = flip elem alphabet</div></div></div><div><br class=""></div><div>****************************** 2)</div><div>Try to avoid so much logic in your encrypt and decrypt functions (nested ‘if’ ’then’ ‘else’ makes it harder to follow). Also, I don’t know why you are having your functions here return an Int and letting your ‘encrypt’ and ‘decrypt’ functions do the work of changing them back into a Char. It seems to me if you give a Char to your encryptChar function, you should get back an encrypted Char. Make these functions below finish their work. I assume you were tripping yourself up with what to return if isAlphabetic is false, but just return the char value unchanged. Then the type signature of these 2 functions below could be Char -> Int -> Char instead</div><div><br class=""></div><div><br class=""></div><div><div style="color: rgb(54, 54, 54); background-color: rgb(255, 255, 255); font-family: Menlo, Monaco, "Courier New", monospace; line-height: 18px; white-space: pre;" class=""><div class=""><span style="color: rgb(99, 99, 36);" class="">encryptChar</span> :: <span style="color: rgb(63, 151, 223);" class="">Char</span> -> <span style="color: rgb(63, 151, 223);" class="">Int</span> -> <span style="color: rgb(63, 151, 223);" class="">Int</span></div><div class="">encryptChar char shift = <span style="color: rgb(157, 78, 150);" class="">if</span> isAlphabetic char <span style="color: rgb(146, 205, 120);" class="">-- Only encrypt A...Z</span></div><div class=""> <span style="color: rgb(157, 78, 150);" class="">then</span> (<span style="color: rgb(157, 78, 150);" class="">if</span> ord char + shift > ord <span style="color: rgb(162, 86, 55);" class="">'Z'</span> <span style="color: rgb(146, 205, 120);" class="">-- "Z" with shift 3 becomes "C" and not "]" </span></div><div class=""> <span style="color: rgb(157, 78, 150);" class="">then</span> ord char + shift - alphabetLength</div><div class=""> <span style="color: rgb(157, 78, 150);" class="">else</span> ord char + shift)</div><div class=""> <span style="color: rgb(157, 78, 150);" class="">else</span> ord char</div><br class=""><div class=""><span style="color: rgb(99, 99, 36);" class="">decryptChar</span> :: <span style="color: rgb(63, 151, 223);" class="">Char</span> -> <span style="color: rgb(63, 151, 223);" class="">Int</span> -> <span style="color: rgb(63, 151, 223);" class="">Int</span></div><div class="">decryptChar char shift = <span style="color: rgb(157, 78, 150);" class="">if</span> isAlphabetic char</div><div class=""> <span style="color: rgb(157, 78, 150);" class="">then</span> (<span style="color: rgb(157, 78, 150);" class="">if</span> ord char - shift < ord <span style="color: rgb(162, 86, 55);" class="">'A'</span></div><div class=""> <span style="color: rgb(157, 78, 150);" class="">then</span> ord char - shift + alphabetLength</div><div class=""> <span style="color: rgb(157, 78, 150);" class="">else</span> ord char - shift)</div><div class=""> <span style="color: rgb(157, 78, 150);" class="">else</span> ord char</div></div></div><div><br class=""></div><div>Also, notice how similar the 2 functions are. These could be combined. The only difference between them is the ‘+’ and ‘-‘ operations switching. Maybe something like the following</div><div><div style="background-color: rgb(255, 255, 255); font-family: Menlo, Monaco, "Courier New", monospace; line-height: 18px; white-space: pre;" class=""><div style="color: rgb(54, 54, 54);" class=""><div style="line-height: 18px;" class=""><div class=""><span style="color: rgb(99, 99, 36);" class="">shiftChar</span> :: <span style="color: rgb(63, 151, 223);" class="">Int</span> -> <span style="color: rgb(63, 151, 223);" class="">Char</span> -> <span style="color: rgb(63, 151, 223);" class="">Char</span></div><div class="">shiftChar n c</div><div class=""> | isAlphabetic c = chr ((((ord c + n) - base) `mod` alphabetLength) + base)</div><div class=""> | otherwise = c</div><div class=""> <span style="color: rgb(63, 151, 223);" class="">where</span> base = <span style="color: rgb(73, 104, 57);" class="">65</span></div></div></div></div></div><div>This version works for negative or positive values of n. So it can be used to encrypt and decrypt. My brackets are a bit ugly, but I’m getting lazy (it’s late here). </div><div>The use of `mod` here (the infix version of the modulo operator, % in some other languages like python) means we can accept arbitrarily large values of n to our function, and it just wraps around. </div><div>We can also accept negative values.</div><div>The pipe syntax ‘|’ is just a neater way of doing if else statements (see guards for more info)</div><div>The 65 here is because ‘ord' is defined for all possible Char values. You’re defining your valid range for this cipher to capital alphabetic characters, which starts at 65:</div><div><br class=""></div><div><div>*Main> fmap ord ['A'..'Z']</div><div>[65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90] — the corresponding number for each capital letter in order.</div><div><br class=""></div><div>If you haven’t come across fmap yet, it’s the same as map for lists, but you can use it on structures other than lists - see Functors later in your Haskell education :) </div><div>basically it applies the function to each of the value in the list here</div><div><br class=""></div><div>************************************ 3)</div><div>Lastly, your encrypt and decrypt functions</div><div><div style="color: rgb(54, 54, 54); background-color: rgb(255, 255, 255); font-family: Menlo, Monaco, "Courier New", monospace; line-height: 18px; white-space: pre;" class=""><div class=""><span style="color: rgb(99, 99, 36);" class="">encrypt</span> :: <span style="color: rgb(63, 151, 223);" class="">String</span> -> <span style="color: rgb(63, 151, 223);" class="">Int</span> -> <span style="color: rgb(63, 151, 223);" class="">String</span></div><div class="">encrypt string shift = [chr (encryptChar (toUpper x) shift) | x <- string] <span style="color: rgb(146, 205, 120);" class="">-- "Loop" through string to encrypt char by char</span></div><br class=""><div class=""><span style="color: rgb(99, 99, 36);" class="">decrypt</span> :: <span style="color: rgb(63, 151, 223);" class="">String</span> -> <span style="color: rgb(63, 151, 223);" class="">Int</span> -> <span style="color: rgb(63, 151, 223);" class="">String</span></div><div class="">decrypt string shift = [chr (decryptChar (toUpper x) shift) | x <- string]</div></div></div></div><div><br class=""></div><div>I like your use of list comprehensions here :) though as mentioned before I don’t think you should be using the chr function here, rather your encryptChar and decryptChar functions should be doing that to the values they return.</div><div><br class=""></div><div>Because I changed other pieces in the code, I’ll show you another way of doing this part now.</div><div><br class=""></div><div><div style="color: rgb(54, 54, 54); background-color: rgb(255, 255, 255); font-family: Menlo, Monaco, "Courier New", monospace; line-height: 18px; white-space: pre;" class=""><div class=""><span style="color: rgb(99, 99, 36);" class="">encrypt'</span> :: <span style="color: rgb(63, 151, 223);" class="">Int</span> -> <span style="color: rgb(63, 151, 223);" class="">String</span> -> <span style="color: rgb(63, 151, 223);" class="">String</span></div><div class="">encrypt' x = fmap (shiftChar x)</div><div class=""> </div><div class=""><span style="color: rgb(99, 99, 36);" class="">decrypt'</span> :: <span style="color: rgb(63, 151, 223);" class="">Int</span> -> <span style="color: rgb(63, 151, 223);" class="">String</span> -> <span style="color: rgb(63, 151, 223);" class="">String</span></div><div class="">decrypt' x = encrypt' (negate x)</div></div></div><div><br class=""></div><div>Here I’m using ‘point free’ style again only referencing the first of the two parameters expected by encrypt’. Because we changed our other function in answer ******2 on this mail, encrypt’ and decrypt’ are just the inverse of each other. So decrypt' is defined in terms of encrypt’ but just negates the number passed to encrypt’. As you might expect, negate just makes positive numbers negative, and negative ones positive. </div><div><br class=""></div><div>In encrypt’ I also use fmap again, because in Haskell a String is just a type alias for a list of Characters ( type String = [Char]), so we are just mapping the function shiftChar, with the number already baked in, to each character in the list of character (string) which will be passed to this function.</div><div><br class=""></div><div>Please find attached my edits to your source file if you wanna see it in full and play around with it.</div><div><br class=""></div><div>Best regards,</div><div>Lord_Luvat</div><div><br class=""></div><div>P.S. Let me know if I’m a bad teacher, or pitching this response at the wrong level :) I’m trying to learn here too.</div><div></div></div></body></html>