Skip to content

Complete BFS-2-1#618

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

Complete BFS-2-1#618
paridhimalviya wants to merge 1 commit into
super30admin:mainfrom
paridhimalviya:main

Conversation

@paridhimalviya
Copy link
Copy Markdown

@super30admin
Copy link
Copy Markdown
Owner

Strengths:

  • You implemented both BFS and DFS approaches, showing a good understanding of graph traversal algorithms.
  • The code is well-commented and structured.

Areas for Improvement:

  1. For the BFS solution:

    • The initial condition should be that time starts at 0, and we should not increment time before processing the first level. The reference solution does not increment time until after processing the first level of neighbors.
    • Remove the condition if (fresh == 0) return time - 1 at the end. Instead, after the BFS loop, if there are any fresh oranges left, return -1; otherwise, return the time. But note: in the reference solution, the time is incremented at the beginning of the while loop for each level, and the initial rotten oranges are processed at time 0. Then, when we process the next level, we increment time to 1 and so on. However, in your code, you increment time at the beginning of the while loop, so the first level (initial rotten oranges) is processed at time=0, but then you increment time to 1 even if no new oranges are rotten. This is incorrect.
    • Actually, in your code, you start with time=0. Then you enter the while loop and immediately increment time to 1. Then you process the initial rotten oranges. This is wrong. The initial rotten oranges should be processed at time=0. Then the next level should be at time=1. So you should increment time after processing each level, but not before processing the first level.
    • Suggested change: initialize time = -1. Then in the while loop, increment time at the beginning. Then process all nodes in the queue at that level. Then after processing the level, if there are new rotten oranges, they are added. Then when the loop ends, if there are no fresh oranges, return time. But if there are fresh oranges, return -1. Alternatively, you can start time=0 and only increment after processing a level that actually rots some oranges.
  2. The custom queue implementation is not necessary. You can use an array and use removeFirst() (though it is O(n)) or use two arrays to simulate a queue. Given the constraints (m,n <=10), it is acceptable.

  3. The DFS approach is not suitable for this problem. Stick to BFS for problems that require level-by-level propagation.

  4. Test your code with the provided examples to ensure correctness.

@super30admin
Copy link
Copy Markdown
Owner

Strengths:

  • You have implemented a BFS solution that is mostly correct and efficient.
  • You have written clean code with comments and structured classes.
  • You attempted to think about alternative approaches (DFS), which shows creativity.

Areas for Improvement:

  1. The BFS solution has a minor issue: after the while loop, you check if (fresh == 0) return time - 1. This line is never reached because if fresh becomes zero, you return inside the loop. So that code is redundant. You should remove it.

  2. In the BFS solution, you initialize time to 0. Then, in the while loop, you increment time at the beginning of the loop. This means that after the first minute, time becomes 1. However, when you process the first level (initial rotten oranges), you are at time 0. Then you process the next level (oranges that rot in the first minute) and set time to 1. This is correct. But when you find that fresh becomes zero during processing, you return time + 1? Actually, you should return time because the current level being processed is causing the rotting, and the time taken is the current time (which has already been incremented). However, note that you incremented time at the start of the while loop for the current level. So when you are processing the first level (initial rotten oranges), time becomes 1. But that is not correct because no time has passed. To fix this, you should not increment time at the beginning of the while loop. Instead, you should increment after processing each level. The reference solution does this: it increments time after processing all nodes in the current level. Your code does:

    while (!queue.isEmpty) {
         let size = queue.size
         time += 1   // This is done at the beginning of the level
         for ... {
             ...
         }
    }
    

    This means that for the initial rotten oranges, you are already setting time to 1. But at time 0, the initial state is there. So you should start with time = -1 and then increment at the beginning of the loop, or start with time=0 and increment at the end. The reference solution starts with time=0 and increments after processing the level. Alternatively, you can avoid incrementing for the initial level.

    Actually, in your code, the initial rotten oranges are enqueued. Then you enter the while loop. You increment time to 1. Then you process the initial rotten oranges. This is incorrect because at time 0, the oranges are already rotten. So the first level you process (initial rotten oranges) should not count as a minute. Therefore, you should not increment time for the first level.

    How to fix: set time = -1 initially, and then in the while loop, increment at the beginning. Then after processing, if there are no fresh oranges, return time. But if no minutes are needed (no fresh oranges), you return 0. Alternatively, you can process the initial level without incrementing time.

    The reference solution does:

    int time = 0;
    ... 
    while(!q.empty()) {
         int size = q.size();
         time++;   // increment at the beginning of the loop
         for(...) {
             ...
         }
    }
    

    But note: the reference solution checks if there are fresh oranges at the start and returns 0 immediately. Then, when it enters the loop, it increments time. So the first minute is correctly counted.

    However, in your code, you have the same structure. So it should be correct. But let's simulate with a simple case: grid = [[2,1]].

    • fresh = 1
    • queue has [ (0,0) ]
    • enter while loop: size=1, time becomes 1.
    • process (0,0): check neighbors: (0,1) is fresh -> rot it, fresh becomes 0, return time=1.
      This is correct: it takes 1 minute.

    So your BFS is correct in this regard.

  3. The DFS solution is not appropriate for this problem. The problem requires simultaneous propagation from multiple sources, which is best handled by BFS. DFS will not give the correct minimum time because it doesn't account for multiple sources. I recommend removing the DFS solution for this problem.

  4. You have implemented a custom queue. While it is good practice, for Swift, you can use an array and use append and removeFirst (though removeFirst is O(n)). Since the grid is small (max 100 cells), it is acceptable. Alternatively, you can use two arrays to simulate a queue without high cost.

  5. In the BFS solution, you have a force unwrap when dequeuing. It is better to use optional binding safely. You did:
    guard let (cr, cc) = queue.dequeue() else { continue }
    This is safe.

  6. The directions array is correctly defined.

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