Считается ли это использование оператора «экземпляра» плохого дизайна?

Question

В одном из моих проектов у меня есть два «объекта передачи данных» Recordtype1 и Recordtype2, которые наследуют от абстрактного класса RecordType.

Я хочу, чтобы оба объекта RecordType обрабатывались одним и тем же классом процессора записи в методе «процесса». Моя первая мысль заключалась в создании общего метода процесса, который делегирует два конкретных метода процесса следующим образом:

public RecordType process(RecordType record){

    if (record instanceof RecordType1)
        return process((RecordType1) record);
    else if (record instanceof RecordType2)
        return process((RecordType2) record);

    throw new IllegalArgumentException(record);
}

public RecordType1 process(RecordType1 record){
    // Specific processing for Record Type 1
}

public RecordType2 process(RecordType2 record){
    // Specific processing for Record Type 2
}

Я читал, что Скотт Мейерс пишет следующее в Эффективный C ++ :

«Каждый раз, когда вы находите, что пишете код формы« Если объект имеет тип T1, то сделайте что -нибудь, но если это типа T2, то сделайте что -нибудь еще ».

Если он прав, ясно, что я должен ударить себя. Я действительно не вижу, как это плохой дизайн (если, конечно, кто -то не подкладывает Recordtype и не добавляет в recordtype3, не добавляя другую линию к общему методу «процесс», который обрабатывает его, создавая тем самым NPE), и альтернативы, которые я могу думать о том, чтобы вовлечь основной удар по конкретной логике обработки в самих классов RecordType, что на самом деле не имеет большого смысла для меня, поскольку теоретически может быть много разных типов обработки, которые я хотел бы выполнить в этих записях.

Может ли кто -нибудь объяснить, почему это можно считать плохим дизайном и предоставить какую -то альтернативу, которая все еще дает ответственность за обработку этих записей в класс «обработки»?

ОБНОВИТЬ:

• Измененный return null к throw new IllegalArgumentException(record);
• Просто чтобы уточнить, существует три причины, по которым простой метод recordtype.process () недостаточно: во -первых, обработка действительно слишком далека от Recordtype, чтобы заслужить свой собственный метод в подклассах Recordtype. Кроме того, существует целый ряд различных типов обработки, которые теоретически могут выполняться различными процессорами. Наконец, RecordType предназначен для того, чтобы быть простым классом DTO с минимальными методами изменения состояния, определенными внутри.

Answer 1

А Посетитель Паттерн обычно используется в таких случаях. Хотя код немного сложнее, но после добавления нового RecordType Подкласс вас иметь Реализуйте логику повсюду, так как она не будет компилироваться иначе. С instanceof Повсюду очень легко пропустить одно или два места.

Пример:

public abstract class RecordType {
    public abstract <T> T accept(RecordTypeVisitor<T> visitor);
}

public interface RecordTypeVisitor<T> {
    T visitOne(RecordType1 recordType);
    T visitTwo(RecordType2 recordType);
}

public class RecordType1 extends RecordType {
    public <T> T accept(RecordTypeVisitor<T> visitor) {
        return visitor.visitOne(this);
    }
}

public class RecordType2 extends RecordType {
    public <T> T accept(RecordTypeVisitor<T> visitor) {
        return visitor.visitTwo(this);
    }
}

Использование (обратите внимание на общий тип возврата):

String result = record.accept(new RecordTypeVisitor<String>() {

    String visitOne(RecordType1 recordType) {
        //processing of RecordType1
        return "Jeden";
    }

    String visitTwo(RecordType2 recordType) {
        //processing of RecordType2
        return "Dwa";
    }

});

Также я бы порекомендовал сделать исключение:

throw new IllegalArgumentException(record);

вместо возвращения null Когда ни один тип не найден.

Answer 2

Мое предложение:

public RecordType process(RecordType record){
    return record.process();
}

public class RecordType
{
    public RecordType process()
    {
        return null;
    }
}

public class RecordType1 extends RecordType
{
    @Override
    public RecordType process()
    {
        ...
    }
}

public class RecordType2 extends RecordType
{
    @Override
    public RecordType process()
    {
        ...
    }
}

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

http://en.wikipedia.org/wiki/double_dispatch

Answer 3

Другим возможным подходом было бы сделать процесс () (или, возможно, назвать его «dosubclassprocess ()», если это разъясняет вещи) абстрактно (в Recordtype) и иметь фактические реализации в подклассах. например

class RecordType {
   protected abstract RecordType doSubclassProcess(RecordType rt);

   public process(RecordType rt) {
     // you can do any prelim or common processing here
     // ...

     // now do subclass specific stuff...
     return doSubclassProcess(rt);
   }
}

class RecordType1 extends RecordType {
   protected RecordType1 doSubclassProcess(RecordType RT) {
      // need a cast, but you are pretty sure it is safe here
      RecordType1 rt1 = (RecordType1) rt;
      // now do what you want to rt
      return rt1;
   }
}

Остерегайтесь пары опечаток - думаю, я исправил их все.

Answer 4

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

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

PS: в обзоре кода я бы пометил return null, потому что это может распространять ошибки, а не сообщать им. Рассмотреть возможность:

RecordType processed = process(new RecordType3());

// many hours later, in a different part of the program

processed.getX(); // "Why is this null sometimes??"

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

Answer 5

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

Другой эффективность. instanceof довольно медленно, по сравнению с другими методами, такими как сравнение с class объект с использованием равенства.

Когда используешь посетитель шаблон, обычно эффективное и элегантное решение используется Map Для картирования между поддержкой class а также Посетитель пример. Большой if ... else Блок с instanceof Проверки были бы очень неэффективными.