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

verifyRemoveSideEffect possible simplification #118

Closed
ivanduka opened this issue Feb 23, 2019 · 2 comments
Closed

verifyRemoveSideEffect possible simplification #118

ivanduka opened this issue Feb 23, 2019 · 2 comments

Comments

@ivanduka
Copy link

Hi Loiane!

Thank you for this excellent book! I enjoy reading it a lot!

The question is regarding hash-table-linear-probing.js:

Can we simplify the function verifyRemoveSideEffect?

It's original version in the file is:

 verifyRemoveSideEffect(key, removedPosition) {
    const hash = this.hashCode(key);
    let index = removedPosition + 1;
    while (this.table[index] != null) {
      const posHash = this.hashCode(this.table[index].key);
      if (posHash <= hash || posHash <= removedPosition) { // QUESTION IS ABOUT THIS LINE
        this.table[removedPosition] = this.table[index];
        delete this.table[index];
        removedPosition = index;
      }
      index++;
    }
  }

However, since hash is always less than removedPosition, can we just check for posHash <= removedPosition instead of posHash <= hash || posHash <= removedPosition?

This way the function would look like:

  verifyRemoveSideEffect(key, removedPosition) {
    const hash = this.hashCode(key);
    let index = removedPosition + 1;
    while (this.table[index] != null) {
      const posHash = this.hashCode(this.table[index].key);
      if (posHash <= removedPosition) { // THIS LINE IS CHANGED
        this.table[removedPosition] = this.table[index];
        delete this.table[index];
        removedPosition = index;
      }
      index++;
    }
  }

I tried this change and it passes all your tests.

Also, this way we do not need const hash = this.hashCode(key); and subsequently key param of the function:

verifyRemoveSideEffect(removedPosition) { // THIS LINE IS CHANGED
    // const hash = this.hashCode(key); - THIS LINE IS REMOVED
    let index = removedPosition + 1;
    while (this.table[index] != null) {
      const posHash = this.hashCode(this.table[index].key);
      if (posHash <= removedPosition) { // THIS LINE IS CHANGED
        this.table[removedPosition] = this.table[index];
        delete this.table[index];
        removedPosition = index;
      }
      index++;
    }
  }

(of course we would need to change the remove function as well, so it calls this.verifyRemoveSideEffect(position); instead of this.verifyRemoveSideEffect(key, position);

With those changes it also passes all tests.

Does it make sense or there are some edge cases that I do not see and they need this check for both possibilities (posHash <= hash || posHash <= removedPosition)?

Thank you again for sharing your wisdom :)

@loiane
Copy link
Owner

loiane commented Feb 28, 2019

Hi @ivanduka. Yes, you can simplify the code. The original idea was also to show how to handle the same algorithm in other languages as well - as in JavaScript we don't have to set the length of the array prior its creation, then we're good!

@ivanduka
Copy link
Author

ivanduka commented Mar 1, 2019

Thank you @loiane !

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

No branches or pull requests

2 participants