Skip to content

Done design-1#2639

Open
pranjay01 wants to merge 1 commit intosuper30admin:masterfrom
pranjay01:master
Open

Done design-1#2639
pranjay01 wants to merge 1 commit intosuper30admin:masterfrom
pranjay01:master

Conversation

@pranjay01
Copy link

No description provided.

@super30admin
Copy link
Owner

Strengths:

  • You have a good understanding of the data structures and the required operations.
  • Your code is well-commented and structured, making it easy to read.
  • For the MinStack problem, your solution is correct and efficient.

Areas for Improvement:

  • For the HashSet problem, the primary array should be initialized to hold references to secondary arrays, not boolean values. Instead of self.primaryArr = [False] * 1001, you should use self.primaryArr = [None] * 1001 to indicate that no secondary array has been created for a bucket. Then, when adding a key, you check if the primary bucket is None, and if so, initialize the secondary array.
  • In the HashSet remove method, you should check if the primary bucket is None (or not initialized) before trying to access the secondary array. Currently, you check if it is truthy (which in Python, an empty list is truthy, but in your case, it's initialized to False). This needs to be adjusted to the correct initialization.
  • For the MinStack, note that when pushing a value that is equal to the current minimum, you should still push it to the minOrderedItems stack. Your code does this correctly because you use >= in the condition. However, you have commented code that seems to suggest uncertainty. It's good to avoid redundant comments.

Specific Fix for HashSet:
Change the initialization to:

def __init__(self):
    self.primaryArr = [None] * 1001

Then in the add method:

def add(self, key: int) -> None:
    primaryHash = self.primaryHash(key)
    if self.primaryArr[primaryHash] is None:
        # Initialize the secondary array for this bucket
        self.primaryArr[primaryHash] = [False] * 1000
    secondaryHash = self.secondaryHash(key)
    self.primaryArr[primaryHash][secondaryHash] = True

Similarly, in remove and contains, check for None instead of False:

def remove(self, key: int) -> None:
    primaryHash = self.primaryHash(key)
    secondaryHash = self.secondaryHash(key)
    if self.primaryArr[primaryHash] is not None:
        self.primaryArr[primaryHash][secondaryHash] = False

def contains(self, key: int) -> bool:
    primaryHash = self.primaryHash(key)
    secondaryHash = self.secondaryHash(key)
    if self.primaryArr[primaryHash] is not None:
        return self.primaryArr[primaryHash][secondaryHash]
    return False

@super30admin
Copy link
Owner

For the MyHashSet solution:

  • Strengths: You have understood the double hashing concept and implemented it with the correct hashing functions (using integer division and modulo). The code is well-commented and structured.
  • Areas for improvement: The primary issue is the initialization of the primary array with booleans. Instead, you should initialize it with None or empty lists to avoid type errors. For example, when a bucket is unused, it should be None (or an empty list) rather than False. Then, in the add method, you can check if the bucket is None to create the secondary array. Also, consider the edge case for the maximum key (1000000): your secondary hash function might return 1000 for key=1000000, which would be out of bounds for a secondary array of size 1000. The reference solution handles this by making the first bucket have 1001 elements.

Suggested correction for MyHashSet:

class MyHashSet:
    def __init__(self):
        self.primaryArr = [None] * 1001   # Initialize with None

    def add(self, key: int) -> None:
        primaryHash = self.primaryHash(key)
        if self.primaryArr[primaryHash] is None:
            # For the first bucket (index 0), we need to handle key=1000000
            if primaryHash == 0:
                self.primaryArr[primaryHash] = [False] * 1001
            else:
                self.primaryArr[primaryHash] = [False] * 1000
        secondaryHash = self.secondaryHash(key)
        self.primaryArr[primaryHash][secondaryHash] = True

    def remove(self, key: int) -> None:
        primaryHash = self.primaryHash(key)
        if self.primaryArr[primaryHash] is None:
            return
        secondaryHash = self.secondaryHash(key)
        self.primaryArr[primaryHash][secondaryHash] = False

    def contains(self, key: int) -> bool:
        primaryHash = self.primaryHash(key)
        if self.primaryArr[primaryHash] is None:
            return False
        secondaryHash = self.secondaryHash(key)
        return self.primaryArr[primaryHash][secondaryHash]
    
    def primaryHash(self, key):
        return key // 1000
    
    def secondaryHash(self, key):
        return key % 1000

For the MinStack solution (optional feedback): It is mostly correct, but you can avoid storing the minItem as a separate variable by always getting it from the top of minOrderedItems. Also, the condition in push should be if self.minItem is None or val <= self.minItem (note: it's better to use is for None comparison). The commented code can be removed.

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

Comments