Skip to content

Completed ComplexityPractice Pair Assignment #2

Open
LauraV-702 wants to merge 9 commits into
auberonedu:mainfrom
LauraV-702:main
Open

Completed ComplexityPractice Pair Assignment #2
LauraV-702 wants to merge 9 commits into
auberonedu:mainfrom
LauraV-702:main

Conversation

@LauraV-702

Copy link
Copy Markdown

Laura V. and Diana K.

@auberonedu auberonedu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job with the complexity analysis! However, the algorithm for your implementation of the final method is flawed, and your tests aren't sufficient to expose this problem. Make sure to write more thorough tests in the future to catch these type of problems 🙂

Comment thread src/Practice.java
Comment on lines 96 to +120
public static int mostCommonSpaceEfficient(int[] nums) {
// TODO: Complete this method with an implementation that runs
// DONE: Complete this method with an implementation that runs
// in O(1) space.
return -1;

// Initialize variables
int mostCommon = nums[0];
int count = 1;

// for-loop iterating through the nums array
for (int i = 0; i < nums.length; i++) {

// Check if this number is the most common, otherwise move to next
if (nums[i] == mostCommon) {
count++;
} else {
count--;
}

// Move to next number
if (count == 0) {
mostCommon = nums[i];
count = 1;
}
}
return mostCommon;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This algorithm only works if we assume that there's one element that appears more than half the time. However, the code will fail for a case like this: {3, 5, 7, 7, 2, 6, 8}. This code will incorrectly claim that 8 is the most common element. Can you think of a new algorithm that properly handles this case?

Comment thread src/PracticeTest.java
Comment on lines +9 to +21
// DONE: Implement tests for Practice.mostCommonTimeEfficient and Practice.mostCommonSpaceEfficient
@Test
void testMostCommonTimeEfficient() {
int[] numArr1 = {1, 2, 3, 2, 4, 2};
System.out.println(Practice.mostCommonTimeEfficient(numArr1)); // 2
}

@Test
void testMostCommonSpaceEfficient() {
int[] numArr2 = {5, 5, 5, 5, 5, 5};
System.out.println(Practice.mostCommonSpaceEfficient(numArr2));
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are good test cases, but they're not sufficient to thoroughly test the function. We'll often need to have several test cases to test each function.

Comment thread src/PracticeTest.java
Comment on lines +18 to +19
int[] numArr2 = {5, 5, 5, 5, 5, 5};
System.out.println(Practice.mostCommonSpaceEfficient(numArr2));

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good edge case of checking how your method handles it if all the numbers are the same, but we should also test a more standard case like {3, 5, 7, 7, 2, 6, 8};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants