Плохая практика менять объект ненапрямую?

Question

[Ben L]

Плохо ли так делать?

class Position {
    private x: number;
    private y: number;
    
    constructor(x: number, y: number) {
        this.to(x, y);
    }
    
    public to(x: number, y: number): void {
        this.x = x;
        this.y = y;
    }

    // ...
}

class Character {
    private name: string;
    private position: Position;
    
    constructor(name: string, position: Position) {
        this.name = name;
        this.position = position;
    }
    
    // ...
}

const p = new Position(0, 0);
const character = new Character('Darth Vader', p);
p.to(10, 10);

Получается я меняю character без того чтобы что то с ним делать. Стоит ли так делать или же нет? Нарушает ли это какой нибудь принцип?

Comments

[Дмитрий]

Скорее всего будет правильнее у персонажа иметь сет метод позиции, и уже из него вызывать to

[Ben L]

Дмитрий, интуитивно мне тоже так кажется, но обяснить не могу.
С другой стороны так даже удобно иногда, не нужно делать слишком много методов для одного класса.
Допустим у класса Character 3 таких свойств у которых по 2-3 метода, значит у Character будет свой методы + еще 6-9 методов.

Answer 1

[DollyPapper]

Нарушаете принцип инкапсуляции как минимум. Сделайте композицию, чтобы в конструктор передавалась начальная позиция персонажа, и метод чтобы менять эту позицию у самого персонажа. Сейчас у вас p
это не свойство персонажа, а разделенное свойство в области видимости, хотя по логике это именно свойство персонажа.

Comments

[Ben L]

DollyPapper, а если вместо того чтобы добавить метод персонажу сделать getter для position и вызвать to?

class Character {
  // ...
  public getPosition(): Position {
    return this.positon;
  }
}

// ...
character.getPosition().to(10, 12);

[DollyPapper]

Во первых вы теперь вообще нарушили логику наименования. Не getPosition, а setPosition нужно.
Если вы обьект класса Position все равно создаете вне объекта персонажа, это в любом случае нарушение инкапсуляции. Да формально теперь мы не изменяем на прямую свойство, а делаем это через сеттер, но свойство данное всё так же глобально, и любой другой программист может спокойно из своего кода поменять позицию вашего персонажа и вы даже не поймете почему это случилось. То что вы сделали сейчас это Агрегация. При уничтожении вашего персонажа, когда например персонаж умрет, и на него больше не будет ссылок в коде GC соберет его. Но данные вашего персонажа все так же будут жить в памяти. Хорошей практикой является - делать свои объекты целостными, чтобы объект имел в себе все данные и методы для работы и сам решал как ему с этими данными поступать . Разделенное свойство имеет место быть например, если вы делаете стратегию и свойство позиции относится не к одному персонажу, а к группе юнитов. То есть при клике мышью мы задаем позицию для каждого отдельного юнита, который сам по себе может иметь собственную позицию, но при добавлении его в группу, должен направится туда же, куда и группа остальных выделенных в группу юнитов. Реализовать это можно например как написал ниже rPman . Ключевой момент который тут стоит понять, это что на данный ваш объект не целостен, и в данной задаче для того чтобы он был таковым предпосылок - нет. Простите если ответ немного сумбурен - жарко :) Если у вас появятся уточняющие вопросы пожалуйста пишите, постараюсь ответить.

[DollyPapper]

Ben L, посмотрите вот этот доклад
Многие критикуют конечно Егора за его радикальные взгляды на ООП, но как по мне, именно так ООП и должен выглядеть и это то куда эта парадигма и должна стремиться. Ключевая концепция которую продвигает Егор, что объект как сущность должен быть целостен, и сам должен содержать в себе и управлять своими данными, а нам должен быть доступен только его интерфейс.

[Ben L]

Во первых вы теперь вообще нарушили логику наименования. Не getPosition, а setPosition нужно.

Не понял почему нарушил??? Я не предлагал добавить новый метод классу Character а через getter получить доступ к его позиции и у нее вызвать метод который меняет x и y.

Хорошей практикой является - делать свои объекты целостными, чтобы объект имел в себе все данные и методы для работы и сам решал как ему с этими данными поступать

В таком случае придется передовать в конструктор только примитивные данные (или иммутабельные объекты) чтобы не было возможно их менять снаружи, а в конструкторе создавать position для персонажа.

Ключевой момент который тут стоит понять, это что на данный ваш объект не целостен

Думаю я не понял правильно что вы имели ввиду под "целостен".

[Ben L]

DollyPapper,
Видел доклады Егора, в теории мне тоже нравится, но на практике не получается их всех применять потому что:
- с одной стороны в теории объект это что то целое и он должен сам решать что делать со своими свойствами, но с другой стороны, чаще всего, все равно приходится что то с ними делать снаружи и для этого нужно делать специальные методы чтобы получить от объекта данные. То есть доступ к данным есть но не через getters.
- потом, чаще всего на практике не получается использовать только имутабильные объекты (о чем тоже много говорится),если объекты маленькие и они меняются не часто то можно, но часто это не так.

Возможно я неправильно понимаю все это (я бы хотел узнать ответы на некоторые вопросы, но кажется на них нет правильного ответа).

[DollyPapper]

Не понял почему нарушил???

Ну да, вы получаете позицию через геттер, и у результата геттера сеттите значение. Это путает.

Думаю я не понял правильно что вы имели ввиду под "целостен".

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

В таком случае придется передовать в конструктор только примитивные данные (или иммутабельные объекты) чтобы не было возможно их менять снаружи, а в конструкторе создавать position для персонажа.

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

а в конструкторе создавать position для персонажа

. Именно. При создании объекта мы передаем ему в конструктор начальную позицию, после чего мутируем через метод сеттер, или to или move или еще как называем метод перемещения.

[Ben L]

DollyPapper,

Почему иммутабельные?

Если переданный объект иммумабилен то не страшно что кто то изменит его.

При создании объекта мы передаем ему в конструктор начальную позицию, после чего мутируем через метод сеттер, или to или move или еще как называем метод перемещения.

Если на Character есть getter к position то возможно менять позицию и без того чтобы использовать character.move. Если getter-a нет, то как использовать местоположение персонажа? К примеру нужна местоположение чтобы рисовать персонажа или чтобы проверить если он столкнулся с чем то?
Создать метод который будет возвращать x и y? А если это что то сложнее чем position? Возвращать clone от position?

[DollyPapper]

Ben L,

Если на Character есть getter к position то возможно менять позицию и без того чтобы использовать character.move. Если getter-a нет, то как использовать местоположение персонажа?

Может я не верно понимаю о чем вы говорите, но для этого и придуманы getter и setter. Один отдает значение, другой его устанавливает. Почему у нас не должно быть getter'a и setter'a? Как вы будете менять позицию через getter? Если то как вы написали выше, то это очень жесткое нарушение инкапсуляции. Вообще говоря геттеры и сеттеры сами по себе её нарушают.

Если переданный объект иммумабилен то не страшно что кто то изменит его.

Если объект иммутабелен, то изменить его никто вообще не сможет, а не "нестрашно". Иначе он не иммутабелен. И вообще, как вы будете менять позицию персонажа, если объект позиции иммутабелен? Создавая на каждое перемещение копию объекта, но зачем нам это в данном случае я не совсем понимаю.
Иммутабельность просто необходима когда у нас есть действительно разделяемые данные между разными сущностями, в нашем случае с персонажем, позиция конкретного персонажа "Darth Vader" это его и только его состояние. Посмотрите на себя как на объект. Ваше положение в пространстве координат вы делите со мной, с Васей, с Машей, с Петей? Нет. Это только ваше состояние, которое принадлежит вам как объекту. Оно в вас инкапсулировано. Почему Дарт Вейдер должен получать своё состояние о нахождении в пространстве из внешнего объекта?

[Ben L]

DollyPapper,

Как вы будете менять позицию через getter?

character.getPosition().to(10, 10);

Answer 2

[rPman]

Согласен с ответом DollyPapper, но формально возможна ситуация когда так можно делать

К примеру, если научить объект Position сообщать о действиях над собой своим овнерам, создав абстрактный класс CanHavePosition с виртуальным методом doMoving который должен быть имплементирован в классе Character (который наследуешь от CanHavePosition) и вызываться на каждое движение Position::to (способы оформления могут другими быть, например наоборот, в конструкторе Character будет вызываться метод init у Position с колбеком, который будет вызываться внутри Position::to) то такое оформление как у вас будет оправдано.

Само собой, нет необходимости так делать всегда, и нет нужды следовать строго прямо гайдлайнам, хотя они и 'написаны кровью и ошметками говнокода', всегда можно найти ситуацию, когда не нужны over engineering.

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