Skip to content

BFS 2-1 Done#629

Open
pranjay01 wants to merge 1 commit into
super30admin:mainfrom
pranjay01:main
Open

BFS 2-1 Done#629
pranjay01 wants to merge 1 commit into
super30admin:mainfrom
pranjay01:main

Conversation

@pranjay01
Copy link
Copy Markdown

No description provided.

@super30admin
Copy link
Copy Markdown
Owner

Oranges getting rotten (Problem1.py)

Your solution is mostly correct and follows the BFS approach. Here are some points for improvement:

  • You can break early when rottenOrangesCount reaches totalOranges during the BFS. This would avoid unnecessary iterations. For example, inside the inner for loop when you rot a new orange, you can check if rottenOrangesCount == totalOranges and then return resultMin+1? But note: because you increment resultMin only after processing the entire level, you cannot return immediately. Alternatively, you can check after adding each new rotten orange: if rottenOrangesCount == totalOranges, then the next minute (which is the current level being processed) is the last. But since you process by level, you should return resultMin+1 only if you are about to process the next level? Actually, in your code, you process a level and then increment the minute. So if during the processing of the current level you rot the last orange, then the current level is the last one that causes rotting. Then, after processing this level, you will increment resultMin. But wait: you only increment resultMin if there is a next level. So if you rot the last orange in the current level, then the queue might be non-empty for the next level? Actually, no: when you rot an orange, you add it to the queue. So if you rot the last orange in the current level, the queue will have those oranges. Then, after the for loop, you check and see that the queue is non-empty and increment resultMin. But that minute is not needed because the rotting already happened in the current minute. So you should return resultMin+1 only if you are in the middle of processing? This is tricky.

Actually, the reference solution breaks early inside the inner loop: when it rots an orange and fresh becomes 0, it returns the current time (which is the minute for the current level). In your code, you don't have a variable "fresh" that tracks the number of fresh oranges. Instead, you track rottenOrangesCount and totalOranges. So you can do the same: when you rot an orange, check if rottenOrangesCount == totalOranges. Then, you can return resultMin+1? But note: at the beginning of the while loop, resultMin is the number of minutes that have passed. The current level you are processing will cause the next minute? Actually, no: in your code, you are processing the current level (which corresponds to minute resultMin). Then, when you rot an orange in this level, it is happening at minute resultMin. So if you rot the last orange in this level, then the answer should be resultMin. But wait: how do you define the minutes? The initial state is minute 0. Then, after one minute, the first level of rotting occurs. So if we are processing the first level (initial rotten oranges), then resultMin is 0. Then, when we process their neighbors, we are simulating the first minute. So if we rot the last orange during the first minute, we should return 1. But in

VERDICT: NEEDS_IMPROVEMENT


Importance of Employee (Problem2.py)

Your solution is correct and efficient, and you have successfully implemented a BFS approach to traverse the hierarchy. Here are some suggestions for improvement:

  1. Variable Naming: The variable name "empoylyesToConsider" is misspelled and should be corrected to something more conventional, like "queue" or "employees_queue". This improves readability.

  2. Redundancy: You are using two dictionaries: employeeSubordinateMap and employeeImportanceMap. However, the employeeImportanceMap already contains the entire employee object, which includes the subordinates. Therefore, employeeSubordinateMap is unnecessary. You can remove it and simply use employeeImportanceMap to get both the importance and the subordinates.

  3. Simplification: Instead of storing the subordinates separately, you can directly access them from the employee object stored in employeeImportanceMap. This would make the code cleaner and reduce memory usage.

  4. Initialization: Consider initializing the dictionaries in a single loop. For each employee, you can store the entire employee object in one dictionary (as in the reference solution), which is sufficient.

Here's a revised version of your code with these improvements:

from collections import deque

class Solution:
    def getImportance(self, employees: List['Employee'], id: int) -> int:
        employee_map = {}
        for employee in employees:
            employee_map[employee.id] = employee
        
        queue = deque([id])
        total_importance = 0
        
        while queue:
            emp_id = queue.popleft()
            emp = employee_map[emp_id]
            total_importance += emp.importance
            for sub_id in emp.subordinates:
                queue.append(sub_id)
        
        return total_importance

This version is more concise and avoids redundant storage.

Overall, your solution is strong and demonstrates a good understanding of the problem. Keep up the good work!

VERDICT: PASS

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