iiegor / xat-server

A Node.js private server of xat.com
6 stars 2 forks source link

Solution for #27 #45

Closed huumanoid closed 7 years ago

huumanoid commented 7 years ago

I've investigated for packet creation methods, which have duplications. I've found, that creation of 'u' packets and 'm' packets for main chat messages are duplicated. packet-builders/user and packet-builders/message have been created and hardcoded packet composings have been replaced. I think this stuff will evolve as project grows. Some refactoring needed. For example: may be we sould impelement separate method for expanding of 'response-to-locate' packet. May be not.

Questions for discuss:

We should implement separate methods for online and offline users.

At first sight, it's convenient to create buildUser method, which can recognize, is user online or not, and make fetch user from db in the second case. However, it requires buildUser to be async. Returning promise from buildUser or even using callback may seems fine, but check out this lines. Here, buildUser should be called in cycle. Making an array of promises and passing them to Promise.all or making recursive calls of callbacks is not the case. That's why sync version of buildUser() have been created. I think, we can implement unified buildUser method even if there is C# async/await syntax in coffee-script (see ES7 async/await for coffeescript

workers.user.buildUser() vs packet-builders/user.buildUser()

In the beginning I've just implemented buildUser method as part of user.coffee package. Later I've decided to extract this packet building stuff in separate files. I don't think is it OK according to our project hierarchy.

huumanoid commented 7 years ago

This changes was merged to my master branch

iiegor commented 7 years ago

@HuuMaNoID You've done a lot of work! Let me check every change that has been made, I'll cherry-pick them and push to master.