LINUX.ORG.RU

private + get/set или public?

 , , ,


0

1

Доброго вечера ЛОР, есть один вопрос, который мне не дает покоя, а именно, какая из кострукций кода считается правильной и почему? Постараюсь объяснить на простом примере.

Возьмем класс Character, который представляет собой игрока в ММО, у класса есть поля mName, mHealth, mEnergy(все публичные) и конструктор, который все это инициализирует.

Есть так же класс Spell со своими полями и у этого класса есть метод execute

public void spellExecute(Character caster, Character target) {
    caster.mEnergy -= this.spellCost;
    target.mHealth -= this.spellDamage
}

Вопрос в чем, насколько хорошее(плохое) данное решение? Часто попадались реализации, где все поля класса Character были private и для работы с ними использовались get / set методы, но, я не понимаю зачем они нужны в данном контексте, дискасс.

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

anonymous ()

Если у тебя Character просто хранилище переменных, как в паттерне Entity Component Systems, то почему бы и нет.

И зачем тебе get/set, может проще в классе Character сделать методы damage и takeEnergy?

crowbar ()
Ответ на: комментарий от crowbar

Да, Character в данном контексте просто хранилище переменных, damage и cost - это все же атрибуты spell'a, которые просто взаимодействуют с Character'ом.

demch0g ()

java

Пиши get/set, привыкай работать руками (на самом деле используй ide).

Kuzy ★★★ ()

Bся суть геттеров и сеттеров в следовании принципу единообразия доступа к полям вашего класса. Если у вас уже все работает через них, то в будущем вы сможете очень просто добавлять в тело сеттеров и геттеров некоторую логику - например валидацию, преобразование типов и прочее. Даже если у вас это обычный контейнер, то тем более этот подход оправдан. К сведению, в скале не нужно вручную создавать эти методы, так как они автоматически будут добавлены в класс, а поле будет приватным. Вот только потом в скала можно переопределить сеттер и геттер, хотя все обращения к ним останутся такими же, как будто вы в джаве обращайтесь к общедоступному полю класса, а не через {get,set}PropertyName().

blan4 ()

Если ты создаешь приватное поле, и к нему делаешь тривиальные сеттеры/геттеры, то это жесточайшее нарушение инкапсуляции и вообще ООП в целом. Другое дело, в джаве иначе нельзя. Тут обычно поступают так: создают приват поле, к нему херачат сеттеры/геттеры. Потом незаюзанное удаляют. И кстати не нужно называть члены класса в цппшном стиле, а то посоны засмеют.

slyjoeh ★★★ ()

Интересно, на какой странице лоровская школота догадается, что заклинание не должно ничего декрементить. Судя по текущим кококо-гетерам, это надолго.

bj ()
Ответ на: комментарий от anonymous

Да какая разница, суть не в этом

Именно в этом, одна половина делает get/set на автомате, «чтобы было», другая просто не знает что можно по другому (и поэтому мечется между get/set и публичными полями). В итоге получается классический javastyle говно-ооп, который не решает никаких проблем.

bj ()

Стандартные аргументы — ты «завтра» захочешь усложнить логику присваивания или получения значения свойств и тебе придётся переписывать клиентов класса, а если сразу сделаешь с геттерами-сеттерами, то не придётся; так принято, так все делают, не выёживайся, самый умный что ли; разнообразные библиотеки работают со свойствами в виде геттеров-сеттеров, а с полями могут быть проблемы.

Моё имхо — нет никаких проблем, но с геттерами-сеттерами точно так же нет никаких проблем, поэтому чтобы не вызывать лишних вопросов — лучше сделать через них. Ну и, имхо, когда таки придётся добавлять геттеры-сеттеры (ибо захочется сделать проверки значений или автовычисляемые значения или кешируемые значения), то код клиента будет выглядеть стрёмно, там поле, тут геттер, там присваивание, тут сеттер. Уж лучше всё в одном стиле.

Правильным является поддержка свойств на уровне языка, но в Java до такого почему то за 20 лет не доросли.

Legioner ★★★★★ ()
Последнее исправление: Legioner (всего исправлений: 1)

С одной стороны если класс просто хранит данные, то оборачивать поля в геттеры/сеттеры смысла немного, но для этого нужно понимать назначение этого класса. Но вот с объектной моделью у тебя как раз проблема, класс Character у тебя вышел хранилищем полей, а Spell классом с логикой. Хотя это Spell должен хранить поля, отвечающие за заклинание, а в Character'е у тебя должен быть метод cast(Spell spell), или что-то типа того.

Из этого моя простоя рекомендация: используй геттеры/сеттеры, не парься, это распространённая практика, и всяким бинутилзам будет проще.

ya-betmen ★★★★★ ()

я не понимаю зачем они нужны в данном контексте, дискасс

Объекты должны использоваться только через интерфейс. В частности твой пример говно, т.к. caster и target должны быть не Character, а соответствующие интерфейсы. Твой вопрос отпадает сам собой.

public void spellExecute(SpellMaker caster, SpellTaker target) {
    caster.castSpell(this)
    target.receiveSpell(this)
}
no-such-file ★★★★★ ()
Ответ на: комментарий от anonymous

Ну как при чем, при том что в основном такое практикуется (практиковалось) в жабе. В c# геттеры-сеттеры скрестили с доступом к членам класса. В плюслах таких фреймворков нет, которые используя рефлекшн(которого в плюсах нет) находят нужные методы (геттеры-сеттеры) для работы.

invy ★★★★★ ()
Последнее исправление: invy (всего исправлений: 1)
Ответ на: комментарий от no-such-file

Ещё лучше через message queue такое реализовать, имхо.
Тогда спелы можно и по сетке гонять через сериализацию без лишних модификаций в коде :)

invy ★★★★★ ()
Ответ на: комментарий от invy

В C# тоже есть практика ставить тупые геттеры-сеттеры, вот уж чего я не понимаю. Выставил голое поле. Захотел поменять — поменял, клиенты перекомпилились и дальше спокойно работают. Единственное, где это может быть оправдано, в библиотеках, чтобы бинарную совместимость не ломать, но не все же библиотеки пишут.

Legioner ★★★★★ ()
Ответ на: комментарий от no-such-file

Да, не прав был, почитал немного, вроде разобрался, теперь сделано так:

    public boolean canCastThisSpell(Spell spell) {
        return this.mEnergy >= spell.getCost();
    }

    public void didUseSpell(Spell spell) {
        this.mEnergy -= spell.getCost();
    }

    public void wasHitBySpell(Spell spell) {
        if (this.mHealth >= spell.getDamage()) {
            this.mHealth -= spell.getDamage();
        } else {
            this.mHealth = 0;
            this.mIsAlive = false;
        }
    }

У спелла:

    public void spellExecute(Character caster, Character target) {
        if (caster.canCastThisSpell(this)) {
            caster.didUseSpell(this);
            target.wasHitBySpell(this);
            System.out.printf("%s attack %s with %s!\n", caster.getName(), target.getName(), this.mSpellname);
        } else {
            System.out.println("Spell can't be casted!\n");
        }
    }

anonymous ()

делай get/set везде.

для примера: вот у тебя есть mHealth. Вроде как это эффективный остатох хп. Через неделю ты придумаешь баф на временное прибавление хп, и получать нужно будет mHealth+buffHealth поинтов. Через месяц вылезет полезный для танков баф с перенаправлением половины дамага в мп, и у цели будет target.mHealth-=this.spellDamage/2; target.mEnergy-=this.spellDamage/2. И так далее. Начнешь переписывать все spell-ы?

arkhnchul ★★ ()
Ответ на: комментарий от anonymous

Чисто стилистически

Обычно названия методов - это глагол в неопределенной форме.

Вместо didUseSpell -> useSpell (потому что именно это тут и происходит)

wasHitBySpell -> receiveSpellDamage

Опять же, как уже писали выше - спелл не кастится сам, следователно execute должен быть методом персонажа. Но тут уже чисто мое имхо, я не вижу полного контекста

Midael ★★★★★ ()
Последнее исправление: Midael (всего исправлений: 2)
Ответ на: комментарий от blan4

Весьма предусмотрительная затея делать что-то на всякий случай - анти-YAGNI такой...

anonymous ()
Ответ на: комментарий от arkhnchul

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

Т.е. объект типа Spell должен содержать например List<Effect> CasterEffects и TargetEffects, а у персонажа, который наследуется от Target или реализует Targettable (целью ведь может быть не только персонаж, но например объекты окружения), должен быть метод applyEffects(List<Effect> effects), который вызывается на себя при касте: applyEffects(spell.CasterEffects), и на цели, при попадании, и учитывает имеющиеся статус эффекты (баффы, дебаффы итп).

Но это так, оффтопик. Если начинать обобщать, когда это не нужно, то можно и с ума сойти :)

Midael ★★★★★ ()
Последнее исправление: Midael (всего исправлений: 4)

Ты ен можешь сделать паблик свойство в интерфейсе

/тхрад

anonymous ()
Ответ на: комментарий от Midael

это было не к касту самого спелла, а в разрезе «сколько на самом деле у цели брать mHealth») Как раз геттером вполне логично выдавать на гора вычисленное число уже со всеми просчитанными списками эффектов, а не накладывать их на одно свойство.

arkhnchul ★★ ()
Ответ на: комментарий от anonymous

Кукарек<кококо> кукарек = new Кукарек<кококо>();

В восьмерке можно сократить до

Кукарек<кококо> кукарек = new Кукарек<>();

maloi ★★★★★ ()

Часто попадались реализации, где все поля класса Character были private и для работы с ними использовались get / set методы, но, я не понимаю зачем они нужны в данном контексте, дискасс.

Естественно надо для более корректной работы. Этот вопрос пережевывался тысячу раз, если не больше. Как будто быдлокодер спрашивает

OkLets ()
Ответ на: комментарий от anonymous

теперь сделано так

Уже лучше, но я же тебе русским языком сказал, что spellExecute должен принимать не Character, а соответствующие интерфейсы, тогда spellExecute будет работать для любого объекта реализующего интерфейс, а не только персонажа - для предметов, групп персонажей, групп предметов, мира в целом. Причём переписывать spellExecute для этого не придётся.

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

no-such-file ★★★★★ ()
Ответ на: комментарий от Legioner

Правильным является поддержка свойств на уровне языка, но в Java до такого почему то за 20 лет не доросли.

Нет же.

Свойства, геттеры, сеттеры являются антипаттернами изменения состояния объекта. В масштабе использования (Use-Case) выбранного объекта они серьёзно усложняют логику приложения, практически всегда предполагая синхронизацию двух и более свойств (ACID на ряд методов), которые не могут быть изменены без взаимосогласования.

Таким образом, лучше избегать get/set/property без реальной нужды в них. Везде использовать методы-функции, при вызове которых объект не «деградирует», всегда находится в детерминированном состоянии и не нуждается в дополнительном «исследовании» (в инспекции в широком смысле) своих свойств.

iZEN ★★★★★ ()
Последнее исправление: iZEN (всего исправлений: 1)
Ответ на: комментарий от iZEN

Это какие то очень специальные приложения без свойств. В 99% приложений есть реальная нужда в простых объектах с геттерами-сеттерами. И никто никуда не деградирует, нет никаких синхронизаций. Это просто наборы информационный полей.

Legioner ★★★★★ ()
Ответ на: комментарий от Legioner

«наборы информационный полей» оформляются в контейнеры, желательно стандартные.

А вообще, вопрос уже поднимался 5 лет назад: Нужны ли вообще геттеры сеттеры в классах?

В частности, мой «вброс»: Нужны ли вообще геттеры сеттеры в классах? (комментарий)

iZEN ★★★★★ ()
Последнее исправление: iZEN (всего исправлений: 2)
Ответ на: комментарий от iZEN

И уже тогда первымже коментом все сказали

Deleted ()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.