Skip to content

Completed Hashing1#2264

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

Completed Hashing1#2264
shinjaneegupta wants to merge 1 commit intosuper30admin:masterfrom
shinjaneegupta:master

Conversation

@shinjaneegupta
Copy link

No description provided.

@super30admin
Copy link
Owner

No specific feedback provided. Refer to GitHub Comment.

@super30admin
Copy link
Owner

Strengths:

  • You have implemented the prime hashing technique correctly for grouping anagrams.
  • The code is clean and well-commented.
  • You have correctly analyzed the time and space complexity for the problems.

Areas for Improvement:

  1. For the Group Anagrams problem, generating the list of primes for every string is inefficient. You should generate the list of primes once and reuse it. You can generate it in the __init__ method or as a class variable.
  2. Using a float for the hash might cause precision issues for very long strings. Consider using an integer instead (though for strings with length up to 100, it might be acceptable in Python with integers, but note that integers can be arbitrarily large in Python, so using integers would be safer). However, the reference solution uses double in C++, which has similar precision issues. Alternatively, you can use a tuple of character counts as the key, which is also common and avoids precision problems.
  3. In the getHash method, you are generating the primes list each time. This should be avoided. For example, you can compute the primes once in the groupAnagrams method and pass it to getHash, or store it as an instance variable.

Suggested improvement for Group Anagrams:

class Solution:
    def __init__(self):
        self.primes = self.first_26_primes()
    
    def first_26_primes(self):
        primes = []
        num = 2
        while len(primes) < 26:
            is_prime = True
            for i in range(2, int(num ** 0.5) + 1):
                if num % i == 0:
                    is_prime = False
                    break
            if is_prime:
                primes.append(num)
            num += 1
        return primes

    def getHash(self, key: str) -> int:
        hash_val = 1
        for ch in key:
            hash_val *= self.primes[ord(ch) - ord('a')]
        return hash_val

    def groupAnagrams(self, strs: List[str]) -> List[List[str]]:
        groupedStrs = defaultdict(list)
        for s in strs:
            key = self.getHash(s)
            groupedStrs[key].append(s)
        return list(groupedStrs.values())

Alternatively, to avoid precision issues entirely, you can use a tuple of character counts:

def groupAnagrams(self, strs: List[str]) -> List[List[str]]:
    groupedStrs = defaultdict(list)
    for s in strs:
        count = [0] * 26
        for c in s:
            count[ord(c) - ord('a')] += 1
        groupedStrs[tuple(count)].append(s)
    return list(groupedStrs.values())

This method is also O(n * k) and avoids any precision problems.

For the other problems, your solutions are good. However, for IsomorphicStrings, the array size of 100 might not be sufficient if the characters extend beyond ASCII 32 to 131. Actually, the problem states that both s and t consist of any valid ASCII character. So you should use a larger array or a dictionary. But in the problem, it says "all characters are in the ASCII set", so your solution might work if the ASCII values are within that range. However, to be safe, you can use dictionaries for both mappings.

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