Some observations on your code:
- You only look at the first (0th) element, and never increment
i, so you don't check any of the subsequent elements for duplication.
- The
length argument is redundant, since we can discover the length of the Array, s, via its .length (or .size) attribute. Using the .length attribute is safer, because it is always valid. For example, duplicate(Array(1, 2, 3, 4, 5, 3), 10) causes an exception (java.lang.ArrayIndexOutOfBoundsException) since the array doesn't have 10 members.
- You initialize
j to have the same value as i, and then compare s(i) == s(j), so you're always going to get a duplicate on the first element, even if there's no duplication in the array.
- You return
result (which indicates whether you've found a result), rather than isDupli (which indicates whether you found a duplicate). Fortunately, we only need one of these, since finding a result is the same as finding a duplicate.
Here's another version that fixes these problems, and simplifies some code:
def duplicate(s: Array[Int]): Boolean = {
val length = s.length
var i = 0 // start counting at value 0
var foundDuplicate = false // Type inference means Scala knows this is Boolean
// Loop through each member until we've found a duplicate.
//
// Note that "!foundDuplicate" is the same as "foundDuplicate == false"
while(i < length && !foundDuplicate) {
// Now compare to each of the remaining elements. Start at the element above i.
var j = i + 1
// Loop through each of the remaining elements.
while(j < length && !foundDuplicate) {
// If we find a match, we're done.
if (s(i) == s(j)) {
foundDuplicate = true
}
// Look at the next j
j += 1
}
// Look at the next i
i += 1
}
// Return the result. If we didn't find anything, this will still be false.
foundDuplicate
}
val myArray1 = Array(1, 2, 3, 4, 5, 6, 2, 8, 9)
val myArray2 = Array(1, 2, 3, 4, 5, 6, 7, 8, 9)
duplicate(myArray1) // Returns true
duplicate(myArray2) // Returns false
However, while this is perfectly OK procedural code, in Scala we can use a much better functional style. (Procedural code uses vars, while loops, etc. which is frowned upon in Scala.)
import scala.annotation.tailrec
def duplicate(s: Array[Int]): Boolean = {
// Helper function to search the array for matches to element at i
@tailrec // Indicates function is tail recursive.
def matchElement(i: Int): Boolean = {
// Helper function to search for a match in remainder of array.
@tailrec
def matchRem(j: Int): Boolean = {
// If j has reached the end of the array, we had no match.
if(j >= s.length) false
// Otherwise, does this element match the target? Match found.
else if (s(i) == s(j)) true
// Otherwise, look at the next element after j.
else matchRem(j + 1) // Recursive call
}
// If this is the last character of the array, then we can't have a match.
if(i >= s.length - 1) false
// Otherwise did we find a duplicate in the remainder of this array?
else if(matchRem(i + 1)) true
// Otherwise, perform another iteration looking at the next element.
else matchElement(i + 1) // Recursive call
}
// Start the ball rolling by looking at for duplicates of the first character.
matchElement(0)
}
This might look rather complex, at first glance, but note that it doesn't have any var declarations or while loops. Of course, this a roll-your-own solution. There are far simpler methods of achieving this using other Array functions.
