Solution

[alyonashunevych]

No description provided.

[mateacademy-ai-mentor]

Review Summary

Great work on implementing the MyEventEmitter class! The code is well-structured and handles most cases correctly. However, a change is needed to fix a bug in the removeAllListeners method.

Currently, the condition if (!eventName) is used to check if an event name was provided. This is too broad, as it incorrectly triggers for valid but falsy event names like an empty string (''). This causes removeAllListeners('') to remove listeners for all events, which is not the intended behavior.

To fix this, please update the condition to be more specific, for example, by checking if (eventName === undefined). This will ensure the method only removes all listeners when no argument is passed. After this small fix, your solution will be ready for approval!

---

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

This implementation of removeAllListeners has a subtle bug. The condition !eventName is too broad. It will evaluate to true for any falsy event name, such as an empty string ('') or the number 0, causing all listeners for all events to be removed. The requirement is to remove all listeners only when no event is specified (i.e., eventName is undefined).

Consider a more specific check, like if (eventName === undefined).

Additionally, the control flow is a bit hard to follow. After this.events = {} is executed, the method continues to the next if statement. It would be clearer to handle the two cases (with and without an eventName) separately, for instance by returning immediately after clearing all events.

[mateacademy-ai-mentor]

Review Summary

Great job on this implementation! You've correctly fixed the removeAllListeners method, and the class is very well-structured. However, there is one subtle but important issue in the emit method that needs to be addressed. If a listener removes another listener for the same event while it's being executed, your current implementation might still call the removed listener. This happens because you are iterating over the original array of listeners. To fix this and fully match the behavior of Node.js's EventEmitter, you should iterate over a copy of the listeners array instead. A simple way to create a copy is with the spread syntax: [...this.events[eventName]]. Once you make this change, your solution will be perfect. Keep up the great work!

---

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

This implementation has a subtle bug. When you iterate over this.events[eventName] directly, if a listener function calls .off() to remove another listener for the same event, that second listener might still be executed because forEach is iterating over the array as it was when emit was first called.

To match the behavior of Node.js's EventEmitter (where a listener removed before it's called won't be executed), you should iterate over a copy of the listeners array. You can create a copy easily using the spread syntax: [...this.events[eventName]].

[mateacademy-ai-mentor]

Review Summary

Excellent work on this iteration! You've successfully addressed the critical issue from the previous review by updating the emit method. Your implementation is now robust and correct.

I am approving your solution as an educational override so you can move forward, but I have one small suggestion to improve readability. In removeAllListeners, the control flow for when eventName is undefined is a bit subtle. You can make the code's intent clearer by adding a return statement right after you clear all events, making the two paths more explicit.

// src/MyEventEmitter.js
removeAllListeners(eventName) {
  if (eventName === undefined) {
    this.events = {};
    return; // This makes the intent clearer
  }
  // ...
}

This is a very strong submission. Keep up the great work!

---

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Footnotes

1. Rate AI review example ↩

[mateacademy-ai-mentor]

While this logic works correctly, it's a bit subtle. When eventName is undefined, the first if block on line 63 executes, and then the second if block on line 67 also executes to cause a return. Consider restructuring this to make the two main paths (clearing all events vs. clearing a specific event) more explicit. For example, you could add a return statement inside the first if block after line 64.