MadeByLoosers / noise-box

Broadcasting noise across the internet since 1972 - NoiseBox allows anyone to play sound clips through another computer...
http://noisebox.co
3 stars 3 forks source link

DRY - controller logic #34

Closed orangespaceman closed 11 years ago

orangespaceman commented 11 years ago

We've now put in quite a bit of logic to display live updates - e.g. to the play queue and usernames

The data itself is stored in the relevant models, but it is updated and accessed in the 'host' controller. (Within the host controller we are listening to changes on the relevant models, and then emitting the updated information via websockets to the front-end, and vice versa).

We now also want this same data to be updatable and accessed by the 'user' controller too.

For example, if you look at the host controller, it has methods such as trackAdded and userChanged that would be useful in the user controller.

Just wondering whether we could introduce an Abstract controller that both the user and host controllers extend, or whether it could be another piece of middleware, or something else entirely.

Opinions?

jedrichards commented 11 years ago

Yup, if you've identified meaningful chunks of functionality that are shared between the controllers then that's definitely time to think about making an abstract controller. I don't think middleware would help in that case, since that's mainly related to code that needs to execute when a client hits a route, whereas what you're talking about is the general flow of socket events, which happen independently of routes.

That said, if the only shared functionality you've identified is that both controllers need to listen to trackAdded and userChanged events on the model, i.e. their only shared functionality is that they bind to the same events (and then handle those events in totally different ways) then there's less benefit in refactoring towards a shared abstract controller. All you'd be saving is having to write model.on(constants.TRACK_ADDED,trackAdded) in each file ...

Unless I misunderstood?