Это использование оператора «instanceof» считается плохим дизайном?

Question

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

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

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, не добавляя еще одну строку к общему методу «Process», который обрабатывает его, создавая таким образом NPE), и альтернативы, которые я могу думать связанные с тем, что накладывают основную нагрузку на конкретную логику обработки внутри самих классов RecordType, что на самом деле не имеет для меня большого смысла, поскольку теоретически может быть много разных типов обработки, которые я бы хотел выполнить на этих записях.

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

UPDATE:

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

Answer 1

В таких случаях обычно используется шаблон Visitor. Хотя код немного сложнее, но после добавления нового подкласса 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() 
    { 
     ... 
    } 
} 

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

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

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

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

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 чеками был бы очень неэффективен.