-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create 1898-maximum-number-of-removable-characters.js #2621
Conversation
Solved maximum-number-of-removable-characters in JS.
} | ||
} | ||
|
||
function isSubSet() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Avoid internal functions that relay on global scope
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always see @neetcode-gh writing nested functions to avoid passing the extra arguments. That is why I also do it. I don't like not passing extra arguments. But I'll fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
s[removable[i]] = -1; | ||
if(isSubSet()) { | ||
k++; | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Avoid else
if (true) continue;
return ...;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a kind reminder that I updated the code as suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aakhtar3, sorry for the delay. I got busy. I have removed the else case as suggested. Please check.
Avoiding else case and defining function outside the main solution function.
var maximumRemovals1 = function(s, p, removable) { | ||
let k = 0; | ||
// removable.reverse(); | ||
s = s.split(''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will increase space complexity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the space complexity to n. But we already had s and p as strings we just used them in an array instead of a string. wouldn't time complexity be the same for that? Before we were using n space for string which we were getting as an argument. Now we use the same number of array elements.
In other words, since we're just converting some string of n char to be an array of n elements. Then, the space complexity is the same as before right? because we were already using space(n) for the strings. Let me know if I'm making a mistake. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sting is immutable primitive data type. You can not modify its state or space in memory.
If you split and convert to char array, it will need new space to allow the list to exist.
If you need to modify the string you can split, but if you need to read by index, you do not need to split and it will lower space complexity.
/* Time O(N + N) | Space O(N) */
for (const char of str.split('')) { ... }
/* Time O(N) | Space O(1) */
for (const char of str) { ... }
/* Time O(N) | Space O(1) */
for (const index in str) {
const char = str.charAt(index);
const char = str[index];
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aakhtar3, I updated the space complexity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can reduce the space O(1) by avoiding the split
and just iterate with for (const char of str)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aakhtar3, but if we don't convert it to the array then how are we going to update a specific char of the string? See line 18.
I did remove the split method on the binary-search solution and used the bitwise operation to find the mid now. Please check.
s[removable[i]] = -1; | ||
if (isSubSet1(s, p)) { | ||
k++; | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aakhtar3, It's done. I removed the else case.
Updating the space complexity for the brute force approach.
Avoiding else case.
let k = 0; | ||
|
||
while (left <= right) { | ||
const mid = Math.floor((left + right) / 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Bitwise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's done, using bitwise operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Waiting for an update. I changed the code as suggested.
var maximumRemovals1 = function(s, p, removable) { | ||
let k = 0; | ||
// removable.reverse(); | ||
s = s.split(''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can reduce the space O(1) by avoiding the split
and just iterate with for (const char of str)
Using bitwise operation to get the middle value. Not converting the strings to array to keep the time complexity O(1).
Solved maximum-number-of-removable-characters in JS.
File(s) Added: 1898-maximum-number-of-removable-characters.js
Language(s) Used: JavaScript
Submission URL: https://leetcode.com/problems/maximum-number-of-removable-characters/submissions/977122834/