anch7 / nodachat Goto Github PK

У тебя подключен debug, но в config.js файле ты используешь console.log.

    app.get("/db", function (req, res, next) {
        db.findAll(function (err, data) {
            if (err) {
                return next(err);
            } else {
                res.json(data);
            }
        });
    });

Имя роута явно не соответствует тому, для чего он предназначен.

Вместо

var $userAvatarImage = $("<img/>").addClass("media-object img-circle").attr("src", avatarUrl).css("max-height", "60px");

лучше

var $userAvatarImage = $("<img/>")
    .addClass("media-object img-circle")
    .attr("src", avatarUrl)
    .css("max-height", "60px");

И таких мест в chat.js файле много. Читабельность кода.

Все папки, относящиеся к серверному коду, стоит сгруппировать в одну папку, например, server. Тем самым у тебя будет четкое разделение на server, public (client) и компоненты (bower, npm), которые будут находиться на одном уровне.

У тебя не везде используется strict mode.

А почему ты хранишь копии всех скриптов в отдельной папке (public/scripts/), хотя у тебя используется Bower и все пакеты сохраняются в bower_components?

Если ты не знаешь, как подключить скрипты из bower_components и поэтому скипировал их, то я бы тебе посоветовал воспользоваться Grunt или Gulp для сборки проекта. Помимо этого ты в случае надобности мог указать Bower-у, куда именно нужно сохранять пакеты (с помощью .bowerrc JSON-файла, http://bower.io/docs/config/).

.chathistory {

Класс следовало назвать chat-history.

    function createParticipantsMessage(connectedUsers) {
        var message = "";
        if (Object.keys(connectedUsers).length === 1) {
            message += "there's 1 participant";
        } else {
            message += "there are " + Object.keys(connectedUsers).length + " participants";
        }
        return message;
    }

Эту функцию можно переписать следующим образом:

    function createParticipantsMessage(connectedUsers) {
        if (Object.keys(connectedUsers).length === 1) {
            return "there's 1 participant";
        }

        return "there are " + Object.keys(connectedUsers).length + " participants";
    }

Не реализована функциональность сессий.

У тебя app передается в sockets.js, хотя там это не нужно.

1. В соседних вкладках можно авторизоваться под разными пользователями (отсутствуют сессии).
2. После обновления страницы пользователю предлагается заново авторизоваться (отсутствуют сессии).
3. Не работает submit на логин-форме.
4. Нет ограничений по минимальной длине и сложности паролей.
5. После авторизации зарегистрированным ранее пользователем показывается null вместо имени в нескольких местах.
6. Дублирование списка пользователей в случае запуска чата в двух вкладках.
7. Надпись there's 1 participant не обновляется после добавления / удаления пользователей чата.
8. Не работает submit на отправку сообщений.
9. Нет уведомлений о том, что пользователю пришло личное сообщение.
10. Есть возможность отправить самому себе личное сообщение.

Именовать события принято с использованием разделителей, обычно это двоеточие. Да и формат имен для всех событий должен быть унифицирован. Например, user:create.

Временами у тебя используются одинарные кавычки, а временами двойные. Это должно быть унифицировано, что также улучшит читабельность кода.

    function saveUser(email, password, callback) {
        db.insert({
            email: email,
            password: password
        }, function(err, newDoc) {
            if (err) {
                callback(err);
            } else {
                callback(null, newDoc);
            }
        });
    }

    function findUser(email, callback) {
        db.find({ email: email }, function(err, docs) {
            if (err) {
                callback(err);
            } else {
                callback(null, docs);
            }
        });
    }

    function findAll(callback) {
        db.find({}, function(err, docs) {
            if (err) {
                callback(err);
            } else {
                callback(null, docs);
            }
        });
    }

У тебя во всех функциях-обработчиках запросов к базе только стандартная логика вызова callback-ов. Поэтому весь код выше можно сократить до следующего:

    function saveUser(email, password, callback) {
        db.insert({
            email: email,
            password: password
        }, callback);
    }

    function findUser(email, callback) {
        db.find({ email: email }, callback);
    }

    function findAll(callback) {
        db.find({}, callback);
    }

У тебя socket.io подключается и инициализируется в config.js файле, потом io-объект передается в sockets.js файл, где он реально нужен, и в routes.js файл, где он не нужен. Поэтому стоило бы всю логику по сокетам вынести в sockets.js, включая инициализацию, тем самым отделив создание и инициализацию Express-приложения от всего остального.

Было бы замечательно попытаться как-то разбить весь клиентский код на несколько смысловых блоков и разнести их по файлам. Это улучшит читабельность.

// development error handler - will print stacktrace
if (app.get("env") === "development") {
    app.use(function(err, req, res, next) {
        res.status(err.status || 500);
        res.render("error", {
            message: err.message,
            error: err
        });
    });
}

// production error handler - no stacktraces leaked to user
app.use(function(err, req, res, next) {
    res.status(err.status || 500);
    res.render("error", {
        message: err.message,
        error: {}
    });
});

Достаточно было бы только одного обработчика, внутри которого и будет нужное условие.

if (data.to === "all") {
    addChatMessage(data, false);
 } else {
    addChatMessage(data, true);
}

Можно переписать так:

var isPrivate = data.to !== "all";
addChatMessage(data, isPrivate);

Или даже так:

addChatMessage(data, data.to !== "all");

Предложил бы тебе разнести контроллеры по отдельным файлам, а в файле routes.js оставить лишь определение роутов.