-
-
Save bkyrlach/3afe03b7298f0383700b to your computer and use it in GitHub Desktop.
Code review for Joel...
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
//In order to use the type paramter A, you must define it right after the method name. | |
//Something like... | |
//def pack[A](...) | |
def pack(input: List[A]): List[List[A]] = { | |
//This seems to be a perfect case for pattern matching? Why not use... | |
// input match { | |
// case Nil => Nil | |
// case h :: t => ... | |
// } | |
if(input == Nil) { | |
Nil | |
} else { | |
// 1. List[...] is a type signature. To build a list, use List(...) | |
// 2. input.filter returns a list as you correctly stated. When you cons that to | |
// a list of lists (that returned by pack) you don't need to wrap it in another | |
// list. | |
List[input.filter(_ == input.head)] :: pack(packHelper(input.tail)) | |
} | |
//Similar to pack, you're forgetting to declare A before you use it. | |
def packHelper(input: List[A]): List[A] { | |
// This seems like another great candidate for pattern matching... | |
if(input == Nil){ | |
Nil | |
} else { | |
//This logic is unsafe, as nth(1, input) doesn't always give back the value you expect. | |
//However, isn't this really just a filter? | |
if(input.head == (nth(1, input)) | |
packHelper(input.tail) | |
} else { | |
input.tail | |
} | |
} | |
} | |
//Same problem as pack and packHelper | |
//Additionally, based on some of the comments below, it seems that | |
//what we really want is to return either Some(A) or Nothing. Lets | |
//discuss that. :) | |
def nth(number: Int, input: List[A]): A = { | |
//This seems like a good match for pattern matching. | |
if(input == Nil) { | |
//Nil is a list, but nth is defined as returning an A. This can't be correct. | |
Nil | |
} else { | |
if(number <= 0) { | |
input.head | |
} else { | |
nth(number - 1, input.tail) | |
} | |
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment