Coderbyte
A JavaScript interview question asked at Google
elisabethgross
・3 min read
Hello and welcome back to Code Review, a series of coding interview challenges and career related content released weekly exclusively on Dev.to. I’m Elisabeth and I’ve been a software engineer for about 4+ years now. I’m passionate about sharing my knowledge, and best tips and tricks when it comes to acing that interview and or just leveling up your coding skills. If you want more content and challenges like these, subscribe to the Coderbyte newsletter here. That’s it for stand up - let’s get to challenge solving!
The Challenge
Write a class, EventEmitter that has three methods: on, emit, and removeListener.
-
on("eventName", callbackFn)- a function that takes aneventNameand acallbackFn, should save the callbackFn to be called when the event witheventNameis emitted. -
emit("eventName", data)- a function that takes aneventNameanddataobject, should call thecallbackFns associated with that event and pass them thedataobject. -
removeListener("eventName", callbackFn)- a function that takeseventNameandcallbackFn, should remove thatcallbackFnfrom the event.
For example:
let superbowl = new EventEmitter()
const cheer = function (eventData) {
console.log('RAAAAAHHHH!!!! Go ' + eventData.scoringTeam)
}
const jeer = function (eventData) {
console.log('BOOOOOO ' + eventData.scoringTeam)
}
superbowl.on('touchdown', cheer)
superbowl.on('touchdown', jeer)
superbowl.emit('touchdown', { scoringTeam: 'Patriots' }) // Both cheer and jeer should have been called with data
superbowl.removeListener('touchdown', jeer)
superbowl.emit('touchdown', { scoringTeam: 'Seahawks' }); // Only cheer should have been called
The solution:
This is a great opportunity to use ES6 classes. In case you haven’t used them before, check out their syntax here. We can start with a basic structure for the class EventEmitter and initialize it with an object events that we will use to track our events.
class EventEmitter {
constructor () {
this.events = {}
}
}
On
Next we can start working on our methods. First up is on. Here is the code for that:
on (eventName, callbackFn) {
if (!this.events[eventName]) {
this.events[eventName] = []
}
this.events[eventName].push(callbackFn)
}
Because functions are first class objects in javascript, which basically means they can be stored in a variable, an object, or an array, we can just push the callback function to an array stored at the key eventName in our events object.
Emit
Now, for our emit function.
emit (eventName, eventData) {
if (!this.events[eventName]) return
this.events[eventName].forEach(fn => fn(eventData))
}
This solution takes advantage of what is called closure in javascript. If you are coding in Javascript in your interview, understanding closure can be vital. A closure is essentially when a function has references to its surrounding state or its lexical environment. You can also think of this as a closure allowing you access to an outer function’s scope from inside an inner function. Using global variables is a great simple example of closure.
Here’s another great example of using closure to track how many times a function was called.
function tracker (fn) {
let numTimesCalled = 0
return function () {
numTimesCalled++
console.log('I was called', numTimesCalled)
return fn()
}
}
function hello () {
console.log('hello')
}
const trackedHello = tracker(hello)
The inner function returned in tracker closes over the variable numTimesCalled and maintains a reference to it for the life of the trackedHello function. Cool stuff huh??
RemoveListener
The removeListener method is probably the easiest of the three. Here is a solution -
removeListener (eventName, callbackFn) {
const idx = this.events[eventName].indexOf(callbackFn)
if (idx === -1) return
this.events[eventName].splice(idx, 1)
}
And that’s the class! Pun intended :) Seeing if you can implement methods that are part of the language is a great way to practice for interviews. See you all next week!
I just joined the DEV team!
Fernando -
Write Cleaner JavaScript Code With .some() And .every() Array Methods
Marc Backes 🥑 -
Building Table Sorting and Pagination in Vue.js - with Async Data
Raymond Camden -
50
49


How to build a side project that will impress future employers
Trending on
Introduction to TypeScript Data Types (Part 1)
How to write Javascript utility functions
Decode a Binary Message using JavaScript
What is the Ternary Operator?
freeCodeCamp Pomodoro Clock 01: React Functional Components and Local State
Do you consider learning Elm?
Resources for Mastering Algorithms & Data Structures
I liked this problem! Didn't read the solution so I could practice it myself. Here's what I came up with:
Edit: Okay, having read the solution, it seems like we did a lot of things similarly! My
removeListenermay not be too performant, though, since it replaces the whole array.Edit2: I believe there's a bug in your
removeListenerthat my code accounted for. If you pass in an event that we never listened to, then attempting to executethis.events[event].indexOf(...)will throw an error becausethis.events[event]will beundefined.I wouldn't do that, mainly because it's unnecessarily slow for anything but the first listener. My version (and the solution) just pushes the new listener to the back of the array, which is an
O(1)operation. Your version uses the slice operator and creates a new array, and assigns that to the old one. That'sO(n).It's dangerous to assume that the theoretical time complexity of any algorithm is going to be the same as the empirical time complexity. Case in point
Except in this case, array assignment is definitely slower than pushing a single element to the back of an existing array.
Also, it seems you didn't actually read the post that you linked:
That's not what we're comparing here. We're comparing
[...array, item]andarray.push(item). The latter is indisputably faster.Regardless of whatever you two decide is a better answer, its always nice to see other ways of solving the same problem! Play nicely :)
That's not so different than a typical implementation:
github.com/Olical/EventEmitter/blo...
Kudos
Good point that will definitely be a bug! Nice catch!
That's a good question, and the solution is really clear👍
I'm not sure I'd call the
emitfunction a closure, it's not returning a function, and it's not being run outside it's lexical scope (although the callbacks are closures themselves I guess)I think we can't call any function a closure. We just can say that some function uses a closure.
emitfunction actually uses closure here (inside forEach callback). I can be wrong)Not the emit function - the event callbacks themself close over the
dataobject and gain access to that themselves!For those using typescript here is a not quite type-safe solution:
Edit
Another, perhaps more interesting implementation of EventEmitter could use
Set,Map, andoptional chaining:Of course, there are trade offs to using Map and Set. For one, the order of calls to listeners is no longer guaranteed. Second, a listener can only be added once. So if you were using those two properties to bookend events using a closure or somesuch then this implementation would break your code.
That looks like an unnecessarily complex and overengineered solution.
Google will love you.
I know this is an unfashionable view these days, but to me this is a perfect example of why you need to be careful with Typescript. This is theoretically type safe, but it's harder to read and maintain, and do you really need this level of type safety in a simple event system like this?
It could probably be done more simply in TS than this, but a bit like Java sometimes it's as if the language itself encourages overengineering.
Hey, I appreciate the feedback but I'm having trouble applying the ideas of necessary complexity and non-over/under-engineering. Would you mind explaining these concepts to me?
If I were to guess, I would probably assume that you mean one or more of the following things are too complicated:
.filter(listener => listener !== fn)I used.filter(notEquals(fn))toArrayinstead of a falsy dictionary check seems abnormal to you.My responses are:
notEqualand move on. Perhaps you disagree, and that's fine. However, I'd call this a stylistic choice instead of an engineering one, as all of the hoisted functions are general and not specific to the Emitter class.toArray. In most of the javascript codebases I work in, the edge cases that cause me the most trouble involve undefined property access and timing issues. The use oftoArraydeals with the first. It ensures (much the way your code did) that we are not iterating overundefinedas if it were an array.There's a lot of visual clutter, and lots of helper functions that aren't really necessary (
toArray,equals, andnotEquals). In particular, there's no point in defining wrappers like this:Typing in this case is also overkill, imo, except arguably for the data object that's getting passed in to the listeners.
Hmm, I agree that inline typing in TypeScript is generally not as nice as something like Haskell
However, I'm not sure what you mean by typing itself being overkill. Do you mean that it isn't useful that the equals function is typed, that the class methods and arguments are typed, or something else?
I only included the utility functions to make the example copy-pastable. Generally, the three used functions would be imported like
import { toArray, notEquals, call } from 'utility/fns';So the visual clutter of those wouldn't be in this file. Additionally, for any production code implementing a general interface like EventEmitter it's likely that the implementation itself would be commented.
Given your advice, here is what you are probably looking for:
I don't really see how this is still unnecessarily complex or overengineered, it's quite literally the same code without some helpful type information.
I'm still trying to understand the need for
equalsandnotEqual. You already have the===operator, which checks both type and value equality. Why wrap that in a function?I used equal and notEqual because === and !== are binary operations that use infix notation. To filter out a listener we start with the naive implementation of
this.events[name].filter(listener => listener !== fn);Describing this with words we might say that we are filtering the array of listeners, removing all listeners that aren't equal to
fn. In the naive version we achieve this by creating an arrow function that closes over thefnvariable and the!==operator. On a slightly more abstract levellistener => listener !== fnis turninglistener !== fninto the functional equivalent of(a => b => a !== b)(fn). This pattern shows up a lot in programming and mathematics. It shows up so much that decidable equality is one of the first things that modern languages and functional libraries implement. That said, I didn't want to start bringing in too much functional programming to a simple challenge like this, so I only brought in the bare minimum, which is a curried strict equality function.Does that answer your question satisfactorily?
EDIT tldr:
!==isn't curried,notEqualis.Again, overengineering. Google engineers do this a lot. You'll know when you try to implement a simple feature in Android and realize that the brilliant minds at Google thought it would be creative to have you create 3-4 boilerplate classes just so you can display a simple list of items.
There was no need for this. It's incredible the lengths to which you went to complicate a simple equality expression—creating not just
notEqualbut also its binary inverse,equal, which doesn't actually get used except in the definition of, you guessed it,notEqual😂.Unnecessary abstraction, followed by an essay justifying it.
It seems like you're setting a line for which abstractions in programming are necessary or unnecessary. Also that those abstractions that are unnecessary are overengineered (this being a perjorative). Lastly, in your opinion, my use of notEqual constitutes an unnecessary abstraction. I'm totally willing to accept that you are correct if you can supply some rigorous model for differentiating necessary from unnecessary abstractions.
The pragmatic thing to do is to first write the code that gets the job done, and only then to consider how the code can be improved.
equalandnotEqualare perfect examples of abstractions that serve no meaningful purpose in your code. They directly substitute one expression with another, and are only used once. That's not grounds for refactoring.I don't really think one has to define a "rigorous model." In my view, it comes down to common sense, and answering one question: Is it worth investing additional time to create an abstraction if something simpler would suffice, while retaining code clarity?
Breaking that down further:
Your code fails on both fronts. My eyes bleed when I see code like this:
The interview question asks for an implementation of an
EventEmitterclass. What you are proposing is that I, with my limited time and resources:Ts,Rs,as, andbs, and introduce unnecessary typing information.equalthat serves no real purpose elsewhere in the codebase, except to be used in its binary inverse,notEquals.notEqualsthat has all the blessings of type safety.notEquals<unknown>(obligatory lol).I'm not an interviewer. But if I were one, I can't imagine I'd be too happy with you wasting my time (and yours) by writing unnecessary code.
I love TypeScript as much as the next dev. I used it during my internship and for a pretty involved web app project in my final semester of school. But this... This is overkill, and more of a pain than a blessing.
Its nice that you were able to make this a productive conversation full of exchanging knowledge. You go team!
Sorry! I got side-tracked with some other projects and haven't been able to devote enough time to reply.
I implemented this version of Emitter in about 3 minutes top to bottom. There was no refactoring. I will admit that I tend to hoist lambdas early and name them. Sometimes this is a mistake, but it's not without good reason that I do it. I make this choice because when debugging I find stack traces with names easier to parse, I also find that using named functions helps me personally when reasoning about more complicated combinator chains.
To speak to the point that
equalsandnotEqualswould only be used once, that's true in this example, but unlikely to be true in a larger code base. I tend to build up a prelude for any language I'm writing in. equal and notEqual (curried functions in polish notation for === and !==) are pretty much always in my prelude.I guess what I'm saying here is that I have real, material reasons for writing code using helper functions. To me, it seems like you're assuming that I have no reasons for defining and using those functions, or that my reasons run afoul of some hard rule about writing "clean code". I didn't always write code this way. I adopted this particular pattern after years of squashing bugs arising from typos or someone not using strict equality.
I agree that code clarity is very important. I'm sure you realized that my request for a rigorous model for necessary abstraction was in jest. I was mainly interested to see if you were aware of any existing code complexity measurements. There are several, from cyclomatic complexity to cognitive complexity. However, even after years of research there is no consensus on a metric that delivers exact or even entirely useful metrics for whether an abstraction is necessary or unnecessary (whether it is too complex or not).
This is all to say that your common sense, or the metric for what makes code clean is not likely to be universal. I'm sorry that I made your eyes bleed with a simple one line, well typed function (which is incredibly useful if you ever have to interface with a backend written in written in C# that supplies
null | undefined | [] | T[]as the valid values for an array). But I'm a little surprised at your vehemence against what reads to me as a simple typescript implementation of EventEmitter.As a senior engineer at the last four places that I've worked I've done a lot of interviewing. In fact, one of my interviewing techniques is to write or type up a bit of code like the EventEmitter implementation above, introduce a few performance hits or bugs, and ask the interviewee to explain, fix, or improve the code. I'm keenly interested in what their thought process is when reviewing. I ask them to justify the changes they make, just like I have with you here.
If I were interviewing you and you responded to me the way you have here, I would be quite concerned that your ego might get in the way of your success as a programmer. What is unnecessary when talking about programming is obstinance, abusive and pejorative language, and belittling others for no reason.
P.S: As an aside, Robert Griesemer, Rob Pike, and Ken Thompson designed Go while at Google with the goal of creating a simple language that could be held completely in the programmer's head. It's quite the opposite of over-engineered. If your view of Google is that of a monolith that only does things one way then you might want to look again.
EDIT: grammar
There is probably a simple way to add a generic to the Emitter class that extends a type dictionary to narrow the types for emitters and listeners of specific keywords. If anyone has interest I can write that up.
There's a couple of things that bother me about this solution, both of them in
emit.First, there's no error handling, so a called function could break the forEach with a throw.
The second is a little more subtle, and I'm not sure if, or how it should be fixed. The object is mutable, and one of the callbacks could change it. Is it canonical to not worry about this? How would you go about fixing it? Deep copy? Freezing? Since it's a parameter, not sure the best way.
Making the eventData immutable is probably not the job of the
EventEmitterThe Node.js EventEmitter doesn't alter the data in its
emitimplementationThat's why I don't know the solution to this. In C++, passing along const references solves it from anything but something very intentional. If it's not immutable (or rather, some callback decides to mutate it) you're going to run into all sorts of potential bugs. Things can change based on the order of the callbacks and on the implementation of the emitter. That just really grates on me and feels wrong.
I wonder if that's part of the point of the question. When I did C++ interviews, one of my standard questions was "implement memcpy". I was more interested in how they handled edge cases than the actual copying.
while I like the content, Google has not been a good place for developers for a while and we should stop idolizing the "anything related to google must be good" label
Couldn't agree more. Idolizing any company is a recipe for disappointment. However, they are KNOWN for the caliber of interview questions they ask so its really nice to see some real questions they've asked in the past!
I would say that the implementation is straight forward, but my brain focused on the events member and how it is being used. I have seen this pattern many times and I am surprised there are no common specialized classes for it.
So, first of all, without changing any types, I would optimize the on (and similarly emit) methods like this:
It's a minor improvement for small numbers, but it hurts me to see repeated key access to the same reference.
Second of all, why not write first a class that we could call Lookup to handle all those string indexed lists?
Now, if events is a Lookup class, the code is cleaner and safer:
In fact, most of the code in your EventEmitter class is actually Lookup code. One could add to the Lookup class a forEach method and you wouldn't even need an EventEmitter class:
Also, being all modern and not ending statements in semicolon feels obnoxious to me. It's probably me being me and old, but I had to say it.
how about doing a filter for removing a listener i.e filtering in objects which need not be removed.
Sorry to poop the party, but I hope google asks better questions in interviews for their sake.
Anyone smell that..? ;) Sometimes you get lucky with an "easier" problem - not the worst thing in the world!
Very good post. Thanks.
PubSub
The bases for p/s, yes
You could also have used a Set, assuming that functions are added only one.
Nice, concise and readable answer to a common JS problem that most of us have ended up implementing in one way or another over the years.
Hi, Elisabeth.
I want to share my solution to your series in the Korean community.
Are there any rules I must follow?
Holy shit. I ask the same question in my interviews. Its create a simple EventBus with fire,listen and remove methods 😅