Здравствуйте, Sorc17, Вы писали:

S>Вот, отдаю на суд тем кому не лень заглянуть в чужой код свою писанину на яваскрипте. Это класс игрока для для одной простенькой РПГшки, в которой пока ничего нельзя делать кроме как ходить по карте и чатится в чате.

Забавно что есть комментарии для строк типа

    /**
     * Имя игрока.
     */
    this.name = name;

но нет для гораздо более неочевидных, где вычисляются корни, что-то делится, умножается, складывается.
Т.е. собственно логика не прокомментирована. А имена переменных лучше вместо комментарий, понятнее назвать

       s = wp2.v * (t - wp1.t)
        S = wp2.diag ? Math.sqrt(2) * CELL_W : CELL_W
        ds = s / S
        dx = wp2.x - wp1.x
        dy = wp2.y - wp1.y
        _dx = ds * dx
        _dy = ds * dy

        this.x = wp1.x + _dx
        this.y = wp1.y + _dy
        newi = Map.getCellIOnMap(this.y + CELL_HH)
        newj = Map.getCellJOnMap(this.x + CELL_HW)
        if (newi != this.i || newj != this.j) {
            this.j = newj
            this.i = newi
            this.pathC = this.pathC.slice(1)
        }

Спасибо все за отзывы! Очень полезно для меня оказалось. Тред сохранил, со временем всё учту и починю

Здравствуйте, _Obelisk_, Вы писали:

_O_>Много комментариев. Лучше использовать говорящие имена перменных/функций и стараться избегать тавтологий в духе
_O_> * Идентификатр карты, на котрой находится игрок.
_O_> this.mapId = mapId

можно поинтересоваться, с какого языка this.mapId переводится как "Идентификатр карты, на котрой находится игрок"?

Здравствуйте, Sorc17, Вы писали:

S>Спасибо все за отзывы! Очень полезно для меня оказалось. Тред сохранил, со временем всё учту и починю

Еще почитай "Совершенный код" МакКоннела, в голове яснее будет. Мастрид для начинающего. Большинство данных тебе советов содержатся в той книге.

Вставлю свои 5 копеек:

// Эту функцию можно вызвать только как конструктор (с new).
if (this.constructor !== arguments.callee) {
  throw new Error("Constructor called like a simple function!");

Также эта проверка не позволит нормально реализовать наследование от вашего класса.

this.nameWidth = null;
...
this.path = null
...
this.pathC = null

Вот эти строки, ИМХО, лишние, это же javascript

// Пройденное расстояние между соседними точками пути, где находится
// игрок.
var s
// Полное расстояние между соседними точками пути, где находится
// игрок.
var S

Это дело вкуса, конечно, но имена переменных одной буквой в разном регистре обычно обфускаторы типа YUI делают, чтобы максимально код запутать

Здравствуйте, vodoo, Вы писали:

V>Вставлю свои 5 копеек:


V>

V>// Эту функцию можно вызвать только как конструктор (с new).
V>if (this.constructor !== arguments.callee) {
V>  throw new Error("Constructor called like a simple function!");
V>

V>Также эта проверка не позволит нормально реализовать наследование от вашего класса.

Хммм. Об этом не подумал


V>

V>this.nameWidth = null;
V>...
V>this.path = null
V>...
V>this.pathC = null
V>

V>Вот эти строки, ИМХО, лишние, это же javascript

Просто хотел, чтобы поля класса были собраны в одном месте. А то потом попробуй сыщи где там в теле какого метода новое поле появляется.

Может, дурацкое замечание, но мне почему-то резануло глаз. Очень странно видеть, чтобы параметры, названные i и j, передавались в обратном алфавитном порядке "j, i".

Тут есть два подхода:
1. Можно рассматривать их как декартовы координаты, тогда надо назвать их x, y (первая координата по ширине, вторая координата по высоте)
2. Можно рассматривать их как индексы в матрице, тогда передавать в порядке i, j (первый индекс — номер строки, второй индекс — номер столбца) При работе с матрицами нумеровать так — общепринятая практика, и то, что первой идет "координата по высоте" не должно смущать.

А у вас смесь двух подходов, что может сильно запутать другого человека, которому придется пользоваться этим кодом.