Completed complexity analysis work with partner#4
Conversation
auberonedu
left a comment
auberonedu
left a comment
There was a problem hiding this comment.
Nice job with the complexity analysis! Unfortunately, there is a bug in the implementation of one of your methods. Make sure to write more thorough tests in the future to catch these issues. See comments below.
| public static int mostCommonSpaceEfficient(int[] nums) { | ||
| // TODO: Complete this method with an implementation that runs | ||
| // in O(1) space. | ||
| return -1; | ||
| int number = 0; | ||
| int count = 0; | ||
| for (int i : nums) { | ||
| int currentNum = i; | ||
| int currentCount = 0; | ||
| for (int j = 1; j < nums.length; j++) { | ||
| if (currentNum == nums[j]) { | ||
| currentCount++; | ||
| } | ||
| } | ||
| if (currentCount > count) { | ||
| count = currentCount; | ||
| number = currentNum; | ||
| } | ||
| } | ||
| return number; | ||
| } |
There was a problem hiding this comment.
Nice algorithm! Unfortunately, there's an implementation bug that causes it to return the incorrect answer in some cases. What would happen if your method was passed an array of length one with a single non-zero element in it? (e.g. {3})
| @Test | ||
| void testMostCommonTimeEfficient() { | ||
| int[] nums = {4 ,4 ,2 ,5 ,1, 7, 3}; | ||
| int commonNum = Practice.mostCommonTimeEfficient(nums); | ||
| assertEquals(4, commonNum); | ||
| } | ||
|
|
||
| @Test | ||
| void testMostCommonSpaceEfficient() { | ||
| int[] nums = {3 ,3 ,4 ,5 ,1, 3, 3}; | ||
| int commonNum = Practice.mostCommonSpaceEfficient(nums); | ||
| assertEquals(3, commonNum); | ||
| } |
There was a problem hiding this comment.
Nice tests! However, we need more tests to have more confidence in our solutions. It would be nice to validate things like how your code works if there's a tie, what if all the elements are the same number, what if there's only a single element in the array, etc. Adding these tests would have helped uncover the bug in your code.
…sts to test for array size of 1 and multiple ties, added logic to handle empty arrays and arrays with one element
3 participants
No description provided.