Skip to content

Finished timeSpacePractice#9

Open
Z1springer wants to merge 5 commits into
auberonedu:mainfrom
mechance782:main
Open

Finished timeSpacePractice#9
Z1springer wants to merge 5 commits into
auberonedu:mainfrom
mechance782:main

Conversation

@Z1springer

Copy link
Copy Markdown

Both methods have been completed and a testcase made for each of them.

@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! I think your final method doesn't quite meet the space complexity requirement though, see comments below.

Comment thread src/Practice.java
Comment on lines +30 to 34
// Time Complexity: O(n)
// Space Complexity: O(n^2)
// Does the 'T' look confusing? Consider refreshing on generic methods
// We'll revisit generics as a class later
public static <T> Map<T, Integer> countFrequencies(T[] array) {

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.

Take another look at space complexity for this one. How big is each entry going to be? How many entries are there going be total?

Comment thread src/Practice.java
// in O(n) time. n = nums.size()
return -1;

HashMap<Integer, Integer> frequencies = new HashMap<>();

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.

Remember to use interface types where appropriate (Map).

Comment thread src/Practice.java
Comment on lines +64 to +75
for (int i = 0; i < nums.length; i++) {
if (frequencies.containsKey(nums[i])) {
int count = frequencies.get(nums[i]);
count++;
frequencies.put(nums[i], count);
} else {
frequencies.put(nums[i], 1);
}
if (frequencies.get(nums[i]) >= highestCount) {
highestCount = frequencies.get(nums[i]);
mostCommon = nums[i];
}

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 logic!

Comment thread src/Practice.java
Comment on lines +101 to +119
Map<Integer, Integer> mostCommonMap = new HashMap<>();
// Keeps track of the most common number
int mostCommonCount = 0;
int mostCommonInt = nums[0];

// Loops over the input array to add each number to the HashMap
// Sets each numbers count to 1
for (int num : nums) {
mostCommonMap.put(num, mostCommonMap.getOrDefault(num, 0));

// Variable that targets the count, that being the number that represents the
// amount of times a number appears in the array
int count = mostCommonMap.get(num);

if (count > mostCommonCount) {
mostCommonCount = count;
mostCommonInt = num;
}
}

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.

I like the idea, but this doesn't get us to our desired space complexity of O(1). Consider how the size of mostCommonMap will increase with our input.

Comment thread src/PracticeTest.java
Comment on lines +14 to +24
@Test
void mostCommonTimeEfficient() {
int[] nums = { 0, 1, 1, 2, 3, 4, 5, 6, 7, 8 };
assertEquals(1, Practice.mostCommonTimeEfficient(nums));
}

@Test
void mostCommonSpaceEfficient() {
int[] nums = { 2, 2, 2, 2, 4, 4, 6, 6, 6, 8, 9, 10 };
assertEquals(2, Practice.mostCommonSpaceEfficient(nums));
}

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.

I like these tests, but there needs to be more of them to give us more confidence that our code is working. What other cases could you consider testing here?

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.

3 participants