Class hierarchy: Is there a cleaner pattern for this?

Question

I'm writing this in ES2015, but this problem can also be taken to other languages.

In my situation, I have a Chat class like this:

// Chat.js
import { socket, config } from "./Util.js";
import User from "./User.js";
class Chat {
    constructor(socket, config) {
        // …
    }
    roomExists(room) {
        // …
    }
    createRoom(room) {
        // …
    }
}
// Make sure to export the same instance to every other module,
// as we only need one chat (resembling a singleton pattern)
export default new Chat(socket, config);

Now, this class makes use of User somewhere in createRoom(). The problem is that the User class needs to make use of the Chat instance that we export:

// User.js
import chat from "./Chat.js";
export default class User {
     join(room) {
         if (chat.roomExists(room)) {
             // …
         }
     }
}

But now we have a dependency loop between Chat.js and User.js. This script won't run. One way to solve this would be to never import Chat.js directly, but do something like this:

// Chat.js
import { socket, config } from "./Util.js";
import User from "./User.js";
class Chat {
    constructor(socket, config) {
        // Pass `this` as a reference so that we can use
        // this chat instance from within each user
        this.userReference = new User(this);
    }
    roomExists(room) {
    }
    createRoom(room) {
    }
}
// No need to export a singleton anymore, as we pass the
// reference to the chat in the User constructor

But now, every other class depends on Chat and must be given a reference to a chat instance once we instantiate it. This isn't clean, either, is it? Chat is now a single point of failure and very hard to exchange later on.

Is there a cleaner way of managing this?

Why would `join` (which probably should be `joinRoom` to avoid confusion with the array method `join`) be a method of `User` rather than `Chat`? `Chat` seems to managing all aspects of the rooms otherwise. — Jason Cust, Jul 26 '15 at 18:24
@JasonCust Because semantically, `User`s should `join` a room, but `Chat`s shouldn't join anything. `Chat` manages `createRoom` since some of the rooms are created by the `Chat` (default rooms). There will also be a `createRoom` method in `User` later on to enable user-made rooms. — Chiru, Jul 26 '15 at 18:36
This "`class` singleton" thing is [definitively an antipattern](https://stackoverflow.com/questions/10406552/is-it-right-to-think-of-a-javascript-function-expression-that-uses-the-new-key-as-static). Don't do it. Create an object literal. — Bergi, Jul 26 '15 at 20:20
@Chiru: You really should pass the room or the chat(room) that the user joins or the chat that the user creates a new room in as an argument to these methods. Don't make Chats singletons. If your application needs only one Chat, create a single instance in your App main. This is exactly the singleton antipattern. — Bergi, Jul 26 '15 at 20:24
I agree, @Bergi, thanks. I have now used an object literal instead. — Chiru, Jul 27 '15 at 01:53

score 1 · Accepted Answer · answered Jul 26 '15 at 20:05

Updated

Clearly a User shouldn't have a Chat . So, chat should look like this

class Chat{
   //other declarations

  addUser(user,room) {}

  createSpecificRoom(user,name){}

  moveUserToRoom(user,newRoom) {}
}

If you want to do things from the user object, we can use double dispatching.

class User{
  joinChat(chat,room){
      chat.addUser(this,room);
  }
}

var chat=new Chat();
var user= new User();
user.joinChat(chat,room);

However, IMO the best thing is to use only Chat to add/remove users. Semantically is its job to keep track of rooms and users. About singletons, well if you're using a framework supporting DI, pretty much any service is a singleton, but you shouldn't care about that. Just inject Chat everywhere you need it.

Class hierarchy: Is there a cleaner pattern for this?

1 Answers1