Генератор кода

от ·13 комментариев

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

Задача

Существует генератор кода на разных языках, реализованный следующим образом:

public class Generator {
    private final int language;

    // Language constants
    public static final int JAVA = 1;
    public static final int C = 2;
    public static final int CPP = 3;

    public Generator(int language) {
        this.language = language;
    }

    public void generateIF() {
        switch (language) {
        case JAVA:<Код, генерирующий Java>
            break;
        case C:<Код, генерирующий C>
            break;
        case CPP:<Код, генерирующий C++>
            break;
        }
    public void generateFOR() {
        switch (language) {
        case JAVA:<Код, генерирующий Java>
            break;
        case C:<Код, генерирующий C>
            break;
        case CPP:<Код, генерирующий C++>
            break;
        }
    }<И так далее>
}

Вопросы

  1. Что не нравится в коде и почему?
  2. Как изменить?

Ожидаемые ответы

  1. Должна не нравиться повторяемость кусков кода и их влияние на производительность. А так же, трудности с расширением подобной реализации на новые языки.
  2. Необходимо выделить общего предка или интерфейс, который будет определять список методов, для генератора каждого языка. Затем для каждого языка реализовать отдельный класс.

Что проверяем

Проверяем умение кандидата работать с ООП и способность видеть несовершенство чужого кода.

  • *занудно*

    Вопрос про производительность спорный 🙂 Если в предлагаемом решении метод не девиртуализуется, то получишь тот же самый check =) Правда, он будет работать за O (1), а не за O (n) от количества вариантов, но в некоторых случаях может быть медленнее.

    А вообще да, ужас.

  • cap

    Хм... Ты хочешь сказать, что вызов виртуального метода в наследнике займет время сравнимое с проходом по switch?

  • Я хочу сказать, что время на dynamic dispatch может быть больше, чем проверка первого условия в switch :]

  • cap

    Похоже надо мне tfm почитать... Ибо таких тонкостей я не в курсе.

    Век живи, век учись 🙂

    Но на стороне исправленного кода всё равно остаётся лучшая читаемость и лучшая расширяемость. Кроме того, твое замечание относится только к Java, на сколько я понимаю.

    Хотя твоё замечание надо занести в примечание и, если кандидат выскажет такие же размышления, брать сразу 🙂

  • Это занудное рассуждение :)) К таким кандидатам надо подозрительно относиться 😉

    Суть в том, что dynamic dispatch в худшем случае сходит в vtable класса и узнает адрес нужного метода: т.е. сделает memory dereference. А статический if может никуда не ходить, а прямо на месте проверить условие. Опять же, vtable в таком случае наверняка скешируются... И опять, хороший девиртуализатор расклеит вызов виртуального метода в вызовы невиртуальных, охраняемых type guard'ами: т.е. по сути тем же switch'ем 🙂 *ещё более занудно* конечно, если switch маленький и разворачивается в цепочку if'ов.

    Так что, как всегда в перформансе — фиг его знает, надо пробовать 🙂

  • zav

    Они там несколько изменили задачку, так что повторяемость кода стала под сильным вопросом. Ну и, в свою очередь, я там порефакторил в Enumeration с функционалом, вынесенным в функции объявленных членов этого энума. Там просто свитча нет, связывание происходит во время Compile time для статически обозначенного языка, либо один раз во время инициализации если язык задается динамически.

  • cap

    А где они поменяли задачку? В доступной сейчас на сайте Яндекса вакансии java девелопера задачка в том же виде.

    Примером с Enumeration не поделитесь? 🙂

  • zav

    Сейчас задачка там выглядит так: 

    
public class CodeGenerator
{
  private int _language;
  static final int JAVA = 0;
  static final int C_PLUS_PLUS = 1;
  static final int PHP = 2;

  CodeGenerator(int language) { _language=language; }

  public String generateCode() throws Exception
  {
     switch(_language) {
         case JAVA:        //return generated java code
         case C_PLUS_PLUS: //return generated C++ code
         case PHP:         //return generated PHP code
     }
     throw new Exception("Bad language");
  }

  private String someCodeRelatedThing() throws Exception // used in generateCode()
  {
     switch(_language) {
         case JAVA:        //return generated java-related stuff
         case C_PLUS_PLUS: //return generated C++-related stuff
         case PHP:         //return generated PHP-related stuff
     }
     throw new Exception("Bad language");
  }	
}

    Я ее порефакторил вот в это: 

    
public enum CodeGenerator
{
  JAVA
  {
      public String generateCode() {
          return "Java Code here\n" + this.someCodeRelatedThing();
      }
  },
  C_PLUS_PLUS
  {
      public String generateCode() {
          return "C++ Code here\n" + this.someCodeRelatedThing();
      }
  },
  PHP
  {
      public String generateCode() {
          return "PHP Code here\n" + this.someCodeRelatedThing();
      }
  };

  public abstract String generateCode();

  String someCodeRelatedThing() // used in generateCode()
  {
      switch (this) {
          case JAVA:        return "generated java-related stuff";
          case C_PLUS_PLUS: return "generated C++-related stuff";
          case PHP:         return "generated PHP-related stuff";
          default:          return "Не бывает, но компилятору надо";
      }
  }
}

    Про плюсы такого подхода могу рассказать, но по-моему они очевидны 🙂

  • zav

    Забыл вставить тэги форматирования — и отступы поехали. Просю поправить.

  • cap

    Fixed

  • cap

    Хм... Честно говоря, не очень понимаю чем задача приведённая мной отличается от того, что Вы запостили с Яндекса.

    Собственно я эту задачу и воспроизводил, своими словами 🙂

    Хотя... Вы имеете в виду, что там два метода всего, а у меня подразумевается куча?

    По поводу решения с енумом... На сколько я понимаю, нечто вроде суперкласса с сабклассами и получится... Только вид сбоку. Да и свитч остался...

    Так что про очевидные плюсы хотелось бы послушать 🙂

  • zav

    Разница в том, что там два принципиальн РАЗНЫХ метода — один публичный — интерфейс, другой приватный — какая-то спрятанная унутре реализация (у меня оно package private, а то из подклассов не подергаешь)

    там свитч остался только для демонстрации как будет выглядеть shared код для всех реализаций единый. Основной код перекочевал в подклассы. А плюсы — в том, что хакер Вася, привыкший, что Javа — это 1 (ведь настоящие кульхацкеры не используют символические имена для каких-то там интов) не наглючит и не привнесет сложноотлавливаемый баг к себе в проект. В енуме можно звать только по символическому имени, плюс при добавлении других языков можно их добавлять в любом порядке, хоть в алфавитном. Опять же, объявлением основной значащей фкнкции абстрактной мы добиваемся, чтобы кульхацкер Петя, дохачив туда еще и дельфи не забыл сделать реализацию кодогенераци. А то не скомпилируется 🙂 Ну и вообще читаемость повышается.

  • cap

    Да, приватный метод я пропустил. Хотя, ИМХО, принципиальной разницы в решение он не вносит.

    То решение, которое предлагаете Вы, на мой взгляд, практически эквивалентно предлагаемому мной. Единственно что, если делать private метод доступным наследникам, то делать его нужно не package private, а protected. Т.к. в случае package private к нему так же получат доступ и другие классы из этого же пакета. А это незачем. Да и свитч там ни к чему 🙂