iSerge / scala-cgdk

Scala language starter pack for Russian AI cup 2014 http://russianaicup.ru/
Other
2 stars 4 forks source link

Убрать `Option`s #6

Closed alexander-myltsev closed 10 years ago

alexander-myltsev commented 10 years ago

MyStrategy имеет дело с не-null объектами. Не думаю, что нужно тормозить нутро создавая промежуточные объекты -- Option.

Например, сделать что-то вроде:

class Puck() { ... }

object Puck {
  final val empty: Puck = null

  implicit class PuckOps(val underline: Puck) extends AnyVal {
    def isEmpty   = underline eq null
    def isDefined = underline ne null
  }
}

Что думаешь?

iSerge commented 10 years ago

Согласен, что Option мешается при работе со стратегией. Как это решать корректно я так толком и не придумал, только завернул в Option те параметры, в которых в Яве просто засовывают null и не парятся. По уму все Option нужно раскрывать и обрабатывать в RemoteProcessClient. Тут нужно договориться какую точку выбрать между двумя крайними позициями:

  1. Завернуть все в Option как сейчас и пускай стратегия сама зарбирается с этим всем барахлом
  2. Сразу кидать exception, потомому как во-первых если что-то не пришло, то скорее всего поломался сервер, а во-вторых Явская стратегия в этой ситуации все-равно упадет с NullPointerException, т.к. на нули никто проверять не будет.

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

Я не понял, как пользоваться object Puck из твоего предложения.

iSerge commented 10 years ago

Кстати, организаторы завтра обещали начать интегрировать наш пакет на сервер. Хорошобы к этому моменту перестать ломать API.

alexander-myltsev commented 10 years ago

Strategy не работает с Option. Если какой-то из Option будет None, то в Strategy отправится null (из orNull в Runner). Зачем тогда вообще замедлять лишними упаковками/распаковками -- непонятно. Т.е. надо делать наобарот: внутри подконтрольной библиотеки все делать с null в погоне за скоростью, а наружу выставлять Option.

Хотя в Strategy.move по-моему нужно передавать уже рафинированные от null данные, чтобы не загромождать move.

alexander-myltsev commented 10 years ago

Сейчас накидаю с Puck.

alexander-myltsev commented 10 years ago

Интересно, вот мы конструируем мир. А шайба вообще может от сервера не прийти (Option[Puck]). И получается, что в move будет мир без шайбы? И это нужно будет проверять в move? Как-то неправославно, как считаешь?

Вообще, данные часто не приходят с сервера? Т.е. насколько неполной будет картина мира у разработчиков на этом пакете, если world: Option[World] с puck == None все таки будет None?

/cc @Russian-AI-Cup-2014

iSerge commented 10 years ago

Мне кажется, что если что-то не пришло, то это какое-то экстраординарное событие, иначе если это штатная ситуация уже бы весь форум был забит сообщениями, что стратегия падает непонятно почему. А в этой ситуации, я думаю, не грешно в RemoteProcessClient выкидывать исключение, как это делается в ensureEnum. Тут еще есть такой момент у нас есть требование от организаторов, что клиенты для разных языков должны максимально соответствовать друг другу. И я посмотрел -- во всех остальных пакетах кидают null. Я понимаю, что это не scala way, но возможно нам тоже так нужно сделать. Я пока нашел только одно место где мне хотелось бы оставить Option -- это Hockeyist.lastActionTick

alexander-myltsev commented 10 years ago

По-хорошему нужно фильтровать те сообщения, которые содержат null, чтобы Strategy с ними не работал.

Вот как я предлагаю сделать: https://github.com/alexander-myltsev/scala-cgdk-1/commit/33ccf2edb4427d4d4a92a370811bdb3649670b7f?diff=split

alexander-myltsev commented 10 years ago

И это, конечно, неверно. А верно так: https://github.com/alexander-myltsev/scala-cgdk-1/compare/iSerge:master...master

iSerge commented 10 years ago

Я все еще не понимаю, чем Puck.empty лучше чем просто null.

alexander-myltsev commented 10 years ago

С одной стороны чистота кода: null не появится нигде, кроме Puck. С другой стороны скорость: не нужны промежуточные Some/None.

iSerge commented 10 years ago

С моей точки зрения это выглядит как если на С мы не хотим видеть NULL, мы делаем #define SMTH NULL, а потом везде сравниваем с указатели с SMTH. Зачем огород городить?

alexander-myltsev commented 10 years ago

В препроцессоре нет проверки типов.

Пример:

    val world1: World = null
    val puck1: Puck = null

    val world2 = World.empty
    val puck2 = Puck.empty

Если с кастом типов еще можно выжить, то такое с null поймать сложнее:

    def meth1(): Puck = {
      null
    }

    def meth2(): World = {
      null
    }

Тогда как с empty будет ошибка при проверке типов:

    def meth3(): Puck = {
      World.empty
    }

Посмотри https://github.com/sirthias/parboiled2. Несовпавшее правило Rule -- это null под капотом, но пользователю никогда null не покажут.

iSerge commented 10 years ago

Ок. Я понял зачем такая акробатика может быть нужна. Как это убережет пользователя когда он попытается читать из Puck.empty?

alexander-myltsev commented 10 years ago

Никак. Пользователю в Strategy не нужно будет работать с Puck.empty -- мы должны все отфильтровать до вызова move. С Puck.empty работем мы, и аккуратно.