Избавляемся от instanceof - PullRequest
       37

Избавляемся от instanceof

8 голосов
/ 03 января 2012

В игре на основе спрайтов, которую я пишу, каждое поле в 2D-сетке содержит стек спрайтов.В основном считается верхний.

В модуле правил игры у меня много такого кода:

public boolean isGameWon(Board board) {
    for (Point point : board.getTargetPoints())
        if(!(board.getTopSpriteAt(point) instanceof Box))
            return false;
    return true;
}

Обновление: //Do something считается, если есть Box сверху каждого Target.Я не понимаю, как это можно сделать, просто добавив doSomething() в Sprite, если только doSomething() не вернет 1, если спрайт является блоком, и 0 в противном случае.(и это было бы точно так же, как instanceof).


Я знаю, instanceof считается вредным, поскольку он убивает идею объектно-ориентированного программирования.

Однако я не уверен, какисправить код в моем случае.Вот некоторые мысли, которые у меня были:

  • Не думаю, что будет проще добавить метод isABox() в интерфейс Sprite.
  • Будетэто поможет, если Box был интерфейсом, чтобы другие классы могли получить ту же привилегию?
  • Должен ли я попытаться сделать что-то необычное, например сопоставление с шаблоном / двойную диспетчеризацию, с шаблонами, подобными посетителю? *
  • Это нормально, что модуль правил работает тесно с типами, просто потому, что он все равно должен знать их семантику?
  • Является ли вся идея шаблона стратегии модуля правил некорректной?
  • Это не такНе имеет смысла встраивать правила в спрайтов, так как тогда все они должны быть изменены при добавлении нового типа.

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

Ответы [ 11 ]

7 голосов
/ 03 января 2012

Использование полиморфизм :

class Sprite {
    ..
    someMethod(){
    //do sprite
    }
    ..
}

class Box extends Sprite {
    ..
    @Overrides
    someMethod(){
    //do box
    }
    ..
}

Итак, вам просто нужно вызвать sprite.someMethod () в вашем примере.

5 голосов
/ 03 января 2012

Instanceof: (почти) всегда вреден

Я посмотрел на все ответы на ваш пост и попытался понять, что вы делаете.И я пришел к выводу, что instanceof - это в точности , что вы хотите, и ваш исходный пример кода был в порядке.

Вы уточнили, что:

  • Вы не нарушаете принцип подстановки Лискова, поскольку ни один из кодов Box не делает недействительным код Sprite.

  • Вы не , разветвляяськод с ответом на instanceof.Вот почему люди говорят, что instanceof плох;потому что люди делают это:

    if(shape instanceof Circle) {
        area = Circle(shape).circleArea();
    } else if(shape instanceof Square) {
        area = Square(shape).squareArea();
    } else if(shape instanceof Triangle) {
        area = Triangle(shape).triangleArea();
    }
    

    Это причина, почему люди избегают instanceof.Но это не то, что вы делаете.

  • Существует один к одному отношение между Box и победой в игре (другие спрайты не могут выигратьигра).Таким образом, вам не нужна дополнительная «победная» абстракция спрайта (потому что коробки == победители).

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

Ответ всех остальных (включая мой собственный) добавляет дополнительный механизм проверки, является ли спрайт боксом.Однако они не добавляют никакой надежности.Фактически вы берете функции, которые уже предоставлены языком, и переопределяете их в своем собственном коде.

Томас Наррос утверждает, что вы должны различать в своем коде между "семантическими типами" и "типами java".Я не согласен.Вы уже установили, что у вас есть тип Java, Box, который подклассы Sprite.Таким образом, у вас уже есть вся необходимая информация.

На мой взгляд, наличие второго независимого механизма, который также сообщает «Я - ящик», нарушает СУХОЙ (не повторяйте себя).Это означает отсутствие двух независимых источников для одной и той же информации.Теперь вам нужно поддерживать перечисление и структуру классов.

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


Золотое правило: Используй голову .Не подчиняйся правилам как неопровержимым фактам.Спросите их, узнайте, почему они там, и согните их, когда это уместно.

3 голосов
/ 03 января 2012

Вот моя попытка. Попробуйте определить перечисление с различными типами Sprite:

class Sprite {
    public enum SpriteType {
         BOX, CAT, BOTTLE, HUMAN, TARGET, /* ... */, SIMPLE;
    }

    public SpriteType getSpriteType(){
       return SIMPLE;
    }
}


class Box extends Sprite {
    @Override
    public SpriteType getSpriteType(){
       return Sprite.SpriteType.BOX;
    }
}

И наконец:

public boolean isGameWon(Board board) {
    for (Point point : board.getTargetPoints())
        if(board.getTopSpriteAt(point).getSpriteType()!=SpriteType.BOX)
            return false;
    return true;
}

Таким образом, вы можете решить проблему необходимости создания метода isATypeX () в Sprite для каждого типа X. Если вам нужен новый тип, вы добавляете новое значение в перечисление, и только правило, которому нужно проверить этот тип, должно будет подтвердить его.

3 голосов
/ 03 января 2012

Базовая перегрузка - путь сюда. Это иерархия классов Sprite, которая должна знать, что и как делать, например:

interface Sprite {
    boolean isCountable();
}


class MyOtherSprite implements Sprite {
    boolean isCountable() {
        return false;
    }
 }

 class Box implements Sprite {
    boolean isCountable() {
        return true;
    }
}

int count = 0;
for (Point point : board.getTargetPoints()) {
    Sprite sprite = board.getTopSpriteAt(point);
    count += sprite.isCountable() ? 1 : 0;
}

EDIT: Ваше редактирование вопроса не меняет принципиально проблему. У вас есть логика, которая применима только к Box. Опять же, инкапсулируйте эту конкретную логику в экземпляр Box (см. Выше). Вы можете пойти дальше и создать универсальный суперкласс для своих спрайтов, который определяет значение по умолчанию для isCountable() (обратите внимание, что метод похож на isBox, но на самом деле лучше с точки зрения дизайна, поскольку для Circle нет смысла есть метод isBox - должен ли Box также содержать метод isCircle?).

1 голос
/ 16 ноября 2013

Как насчет использования посетителя.

Каждая точка наследует метод acceptBoxDetection:

boolean acceptBoxDetection(Visitor boxDetector){
           return boxDetector.visit(this);
}

and then Visitor does:

boolean visit(Box box){
        return true;
}

boolean visit(OtherStuff other){
        return false;
}
1 голос
/ 03 января 2012

То, что вы действительно тестируете здесь,

Может ли игрок выиграть игру с этим Sprite в верхней части доски?

Поэтому я предлагаю следующие имена:

public boolean isGameWon(Board board) {
    for (Point point : board.getTargetPoints())
        if(!board.getTopSpriteAt(point).isWinningSprite())
            return false;
    return true;
}

Абсолютно бессмысленно иметь функцию isBox. Никак нет. Вы также можете использовать instanceof.

Но если Box, Bottle и Target являются выигрышными фишками, тогда вы можете получить их все

class Box {
    public override bool isWinningSprite() { return true; }
}

Затем вы можете добавить другой тип "выигрышного" спрайта без изменения функции isGameWon.

1 голос
/ 03 января 2012

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

public int count(Class type) {
    int count = 0;
    for (Point point : board.getTargetPoints())
        if(type.isAssignable(board.getTopSpriteAt(point)))
            count++;
    return count;
}

Я подозреваю, что вы действительно хотите это

public boolean isAllBoxes() {
    for (Point point : board.getTargetPoints())
        if(!board.getTopSpriteAt(point).isABox())
            return false;
    return true;
}
1 голос
/ 03 января 2012

В основном вместо

if (sprite instanceof Box)
    // Do something

Используйте

sprite.doSomething()

, где doSomething() определяется в Sprite и переопределяется в Box.

Если вы хотите, чтобы эти правила были отделены от иерархии классов Sprite, вы можете переместить их в отдельный Rules класс (или интерфейс), где Sprite имеет метод getRules(), а подклассы возвращают различные реализации.Это еще больше повысит гибкость, поскольку позволяет объектам одного и того же подкласса Sprite вести себя по-разному.

0 голосов
/ 03 января 2012

Я думаю, что люди предложили вам правильно. Ваш doSomething() может выглядеть так:

class Sprite {
    public int doSomething(int cnt){
       return cnt;
    }
}

class Box extends Sprite {
    @Override
    public int doSomething(int cnt){
       return cnt + 1;
    }
}

Итак, в своем исходном коде вы можете сделать это:

int cnt = 0;
for (Point point : board.getTargetPoints()) {
    Sprite sprite = board.getTopSpriteAt(point)
    cnt = sprite.doSomething(cnt);
}

Или же вы можете достичь той же цели с тем, что предложили, но это может потребовать дополнительных вычислений за цикл.

class Sprite {
    public boolean isBox() {
        return false;
    }
}

class Box extends Sprite {
    @Override
    public boolean isBox(){
       return true;
    }
}
0 голосов
/ 03 января 2012

Когда вы говорите «стек» и «считается верхний», не могли бы вы просто взять верхний и что-то с ним сделать?

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