Skip to content
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

bug in chapter 1 question 1 solution 2 #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

noorgrewal
Copy link

Hello,

I hope you are doing well.

I noticed a bug in the solution code for ch1-q1-solution2, specifically the function hasUniqueCharactersSort(). We cannot sort a string directly in Javascript - we have to convert the string to an array and then perform the sorting operation on it.

I have fixed the bug. Also, I have attached 2 screenshots below that show the reproduced bug and that the fix works.

Thank you for providing these solutions. Overall, they are very helpful : )

Best,
Noor

Before the fix:
screen shot 2018-02-24 at 6 24 20 pm

After the fix:
screen shot 2018-02-24 at 6 43 48 pm

@Carr1005
Copy link

Carr1005 commented Mar 4, 2018

It did split in the ch1-q1.spec.js:

[
      'abcdefghi',
      'jklpoiuqwerzxcvmnsadf',
      '1234567890',
      'AaBbCcDdeFg1234567890(*&^%$#@!)'
    ].forEach(arg => {

      it(`returns true for unique string: '${arg}'`, function() {
        expect(func(arg.split(''))).to.be.true;
      });

    });

In fact in this repository, many test code ([ch\d-q\d].spec.js) involving string manipulation split the primitive string into an array of string, then pass it as argument into function (in [ch\d-q\d].js): func(arg.split('')).
String in JavaScript is immutable, but Array is on the contrary. It's interesting and we need to be careful to discuss the space complexity of the method. Here, if we assume the input is already in 'array form', then we could say that the space complexity is O(1), but if the question indeed give us 'inputString' as our start point, in my opinion, for usage of sort(), split() takes inevitable O(n) for new array.
Another similar situation is in ch1-q3.js:

    url[j] = '0';
    url[--j] = '2';
    url[--j] = '%';

This kind of assignment operation is only available to array, if we assume the input is already an array, the space complexity could be O(1), but if the input we have is a primitive string, I think it should be O(n) because of splitting.

@kinzhao
Copy link

kinzhao commented Jan 9, 2025

If the input is indeed a string and not an array. The worse case complexity would be O(n^2) as sort would have dependency on an array(a data structure).

function isUnique(str) {
    for(let i=0; i<str.length; ++i) {
        for(let j=i+1; j<str.length; ++j) {
            if(str[i] === str[j]) return false;
        }
    }

    return true;
}

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