ну вот, какие замечания будут по рефакторингу (постом выше)?

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

Если интересно, то вот решение в функциональном стиле, в котором я не могу ни к чему придраться. Хотя кое-что мне и не понятно. Например, почему автору так не нравятся if-ы из-за чего он поместил последовательность основных действий в for.

>Видимо, о Прологе вы знаете еще меньше, чем о функциональном программировании.

Эксперты вернулись в тред. Сча прочитают мне лекцию что в прологе нет паттерн матчинга.

> остался от предидущей версии когда не было никаких схем автентикации

facepalm.

Как я и говорил - все понятно. Понятный if непонятный pattern matching.

(Вы говорите много, но не всегда правильно ^^^ и не всегда понятно ^^^. Так что это не объективный критерий оценки истины).

pattern matching не может быть непонятнее - как минимум из-за того, что код для разбора 6 вариантов в случае с паттернматчингом будет просто короче и без кучи ненужного мусора.

> ошибка в виде залогиненого юзера когда не надо, легко вкрадывается и фиг отслеживается без отладчика что гдето в каком-то if ! (not) забыли

фиг отслеживается без отладчика

Я связан с железом и разработкой embedded SW, включая hard realtime, где задержки считаются сотнями тактов. Дебаггер помогает только в самых простых сценариях. В других - рулят голова, анализ кода, анализ сценариев + минимальное количество логирования (printf - медленная вещь, портящая тайминги и мешающая дебажить).

Ваш пример с отладчиком вызывает лишь улыбку :) Хотя чего это я, неквалифицированные кодеры то же нужны для быстрого написания кучи неответсвенного быдлокода.

> Эксперты вернулись в тред.

Ну куда тут многим до ваших умений, знаний и вообще. Выловить пропущенное условие в операторе if() только с помощью отладчика - это сильное мастерство. По-другому-то никак. В коде же не видно :)

>Ваш пример с отладчиком вызывает лишь улыбку :)

Ваши способности к чтению вызывают лишь грусть.

Если читать внимательнее - так много можно понять.

>Выловить пропущенное условие в операторе if() только с помощью отладчика - это сильное мастерство. По-другому-то никак. В коде же не видно :)

Точно точно. Ведь я же утверждал что отладчики рулят а все другие способы гавно.

> Если интересно, то вот решение в функциональном стиле, в котором я не могу ни к чему придраться.

Автор, редиска, вместо того, чтобы прочитать документацию к Either, занялся введением алиасов, да ещё и наоборот. Сейчас придёт www_linux_org_ru и скажет: «Ага! Я же говорил.»

Хотя кое-что мне и не понятно. Например, почему автору так не нравятся if-ы из-за чего он поместил последовательность основных действий в for.

Это сахар типа do-нотации Хаскелла для использования Either в монадном стиле. Как раз это мне у автора понравилось. Перепиши на if-ах и почувствуй разницу.

>Если интересно, то вот решение в функциональном стиле, в котором я не могу ни к чему придраться. Хотя кое-что мне и не понятно. Например, почему автору так не нравятся if-ы из-за чего он поместил последовательность основных действий в fo

Да просто жесть! Ты не нашел к чему придраться хотя даже не понял что оно делает. Гварды его испугали а for comprehension коотрый легко может сгенерировать MatchError, потому что то как оно работает - это scala compiler magic - он не нашел к чему придраться.

> Ну куда тут многим до ваших умений, знаний и вообще. Выловить пропущенное условие в операторе if() только с помощью отладчика - это сильное мастерство. По-другому-то никак. В коде же не видно :)

Смотрим в книгу, видим фигу? Пост был о том, что код должен писаться так, чтобы ошибки отлавливались компилятором, а не отладчиком.

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

это был *минимальный* рефакторинг, естественно можно и нужно сделать получше

Если интересно, то вот решение в функциональном стиле, в котором я не могу ни к чему придраться.

совершенно неприемлемо

1. куча функций одноразового использования вместо комментариев

2. уж если настолько широко все перекроено, что сделаны новые функции, надо было перенсти часть кода в класс АuthScheme (или хотя бы извиниться насчет отсутствия этого)

3. ну и длина 50 строк вместо моих 30 — канцелярит во всей красе (я тут не говорю что я написал хорошо, еще порефакторю немного)

из положительного видно только уже предалагавшееся здесь Success/Failure вместе с разделением, хотя мне они пока чем-то не очень нравятся

> for comprehension коотрый легко может сгенерировать MatchError, потому что то как оно работает - это scala compiler magic

Это просто сахар для map, flatMap и filter. А flatMap, который задействован в данном случае - это монадный bind.

>Автор, редиска, вместо того, чтобы прочитать документацию к Either, занялся введением алиасов

Угу. Что смотриться весьма неконсистентно с последующим матчем на Left и Right.

C forом неплохо - но люди типа eao197 в это месте должны были вообще подвиснуть, если их гварды пугают.

А методы вида

def handlePetrivkaAuthSchemeLogin(user: UserInfo, password: String): LoginResult =
if( user.passwordMatches(password) ) Success(user)
else Failure(«Authentication failed»)

Мне все равно не нравится.

К стати Circumflex тут не подойдет. Автор этого не знает, с него спросу нет, но circumflex это httpшная либа общего назначения - и работает она в общем с текстами. У меня использована либа специализированная REST/AJAXовская. Она автоматически генерирует ajax-rpc, и помимо возврата всегочегопопало осущестляет прямой и обратный биндинг JSON - то есть апи заточено для удобства работы из жаваскрипт. А по апи circumflex такого нельзя сделать в принципе - там надо солнце руками закатывать «с другой стороны».

>Это просто сахар для map, flatMap и filter. А flatMap, который задействован в данном случае - это монадный bind.

Я знаю. Они этого не знают «и тем не менее».

По поводу MatchError - я вот что имел ввиду: http://scala-programming-language.1934581.n4.nabble.com/Is-MatchError-becomin...

Этот момент неприятен и его имхо надо разрулить в будущих версиях скалы.

> Точно точно. Ведь я же утверждал что отладчики рулят а все другие способы гавно.

Вот цитата из вашего поста:

легко вкрадывается и фиг отслеживается без отладчика что гдето в каком-то if ! (not) забыли.

Здесь написанно, что «фиг отслеживается без отладчика». Это означает, что необходим отладчик для выявления «забыли в каком-то if ! (not)», а иначе - фиг.

> совершенно неприемлемо

1. куча функций одноразового использования вместо комментариев

Мартин Фаулер с вами категорически не согласен! С его доводами по рефакторингу можно (и следует) ознакомится с помощью его книги с одноименным названием. Возможно, вы тогда пересмотрите свои «неприемлимо»

> > Ну куда тут многим до ваших умений, знаний и вообще. Выловить пропущенное условие в операторе if() только с помощью отладчика - это сильное мастерство. По-другому-то никак. В коде же не видно :)

Смотрим в книгу, видим фигу? Пост был о том, что код должен писаться так, чтобы ошибки отлавливались компилятором, а не отладчиком.

Спасибо, кэп! А я уж подумал, что пост был о том, как легче писать код с меньшим числом ошибок.

Не просветите, как заставить компилятор искать ошибки в логике программы? Ну там коньюнкция вместо дизъюнкции или забытый not? Или уможение вместо сложения? Мне правдо очень сильно это интересно.

Синтаксические и часть семантических он найдёт. А вот что делать с логическими? С кривым кодом?

>Мартин Фаулер с вами категорически не согласен!

Мартин Фаулер - не истина в последней инстанции.

Он - фаза которую надо пережить.

> У тебя дефолтное поведение _пустить_. Это имхо недопустимо для кода связанного с безопасностью.

тут согласен; видимо придется все же делать Either — чем оно мне не нравится, объяснить не могу (пока что)

Между if( ! AccountsStorage.contains(login) ) и f( user.inactive ) надо получение самого юзера.

это я совсем затупил без компилятора, юзер просто неопределен

Чего я понять не могу - так это чего вы все пытаетесь избавиться от паттерн матчинга.

1. не «вы», а «ты»

2. не пытаюсь избавиться, он там зачастую так же бесполезен, как в if( login == null || password == null )

кстати, у тебя похоже получение юзера совсем не консистентно со скаловскими коллециями?

> Сейчас придёт www_linux_org_ru и скажет: «Ага! Я же говорил.»

не угадал

> Мартин Фаулер с вами категорически не согласен! С его доводами по рефакторингу можно (и следует) ознакомится с помощью его книги с одноименным названием. Возможно, вы тогда пересмотрите свои «неприемлимо»

тем хуже для него, если правда так

кстати, можно цитату с его мнением насчет функций одноразового использования вместо комментариев?

>кстати, у тебя похоже получение юзера совсем не консистентно со скаловскими коллециями?

В смысле что сторадж не имплементит какой нить List ? Угу - не имплементит.

Тут надо понимать что это не хобби проект на который можно убить сколько угодно времени - потому трудно ожидать что я его отполирую для удовлетворения всем Productа, Seq, Mutable и прочим - сравнение тут уже бессмысленно потому что в моем коде я просто за 10 минут написал приемлемо работающий вариант - а тут открыли такой симпозиум по всему интернету - что своро в MIT на лекциях начнут разибраться, а то и курсачи писать по тем строчкам:)

1. куча функций одноразового использования вместо комментариев

ИМХО, дело вкуса. Заинлайнить их, если не нравится.

2. уж если настолько широко все перекроено, что сделаны новые функции, надо было перенсти часть кода в класс АuthScheme (или хотя бы извиниться насчет отсутствия этого)

ООП головного мозга? Зачем?

3. ну и длина 50 строк вместо моих 30 — канцелярит во всей красе (я тут не говорю что я написал хорошо, еще порефакторю немного)

Следствие п.1. Тем не менее, это не бойлерплейт. И в твоём коде существенный недостаток - это login by default, как уже отметил r.

из положительного видно только уже предалагавшееся здесь Success/Failure вместе с разделением, хотя мне они пока чем-то не очень нравятся

Так зато четко видно, что функция аутентификация завершается 2-мя вариантами. И саму аутентификацию можно вынести в «почти чистую» функцию (если допустить, что функции проверки пароля состояния не имеют). Можно её тестировать отдельно, не заморачиваясь с моками для respond и session.

К стати автор говорящий о недостаточной скаластости моего кода и советующий скаластый circumflex - посмотрел бы сначала вовнутрь этого «скаластого» circumflexа.

def flush_!(): Nothing = {
  flush()
  throw new ResponseSentException
}

  def apply() = try {
    val matches = _matchers.flatMap(m => m.apply match {
      case Some(matches: Seq[MatchResult]) => matches
      case _ => throw new MatchError
    })
    if (matches.size > 0) Some(matches)
    else None
  } catch {
    case e: MatchError => None
  }

Ага да.

его написал рубист - что видно по таким мелочам как:

def secure_?() = raw.isSecure def userInRole_?(role: String): Boolean = raw.isUserInRole(role)

> Не просветите, как заставить компилятор искать ошибки в логике программы? Ну там коньюнкция вместо дизъюнкции или забытый not? Или уможение вместо сложения? Мне правдо очень сильно это интересно.

Синтаксические и часть семантических он найдёт. А вот что делать с логическими? С кривым кодом?

Можно проверять довольно многое. Например, автоматически доказывать, что алгоритм сортировки действительно сортирует. Если это правда очень сильно интересно, можешь посмотреть в сторону coq и depentent types.

> > Синтаксические и часть семантических он найдёт. А вот что делать с логическими? С кривым кодом?

Можно проверять довольно многое. Например, автоматически доказывать, что алгоритм сортировки действительно сортирует. Если это правда очень сильно интересно, можешь посмотреть в сторону coq и depentent types.

Я вкурсе. Только мы говорили про конкретный пример. И там не было ни coq, ни видимости dependent types.

> кстати, можно цитату с его мнением насчет функций одноразового использования вместо комментариев?

Я не вожу с собой всю библиотеку книг и не использую «рефакторинг» как настольную библию ;)

Попадётся на глаза - поищу.

По памяти было что-то вроде: «если вам хочется добавить коментарий - значит вы что-то делаете не так. Лучше выделить часть кода в короткий метод(функцию) с подходящим названием».

> По поводу MatchError - я вот что имел ввиду: http://scala-programming-language.1934581.n4.nabble.com/Is-MatchError-becomin...

Я так понял, эта проблема относится к for только постольку, поскольку там может происходить pattern matching при деконструкции. Но штука неприятная, да.

>Это означает, что необходим отладчик для выявления «забыли в каком-то if ! (not)», а иначе - фиг.

Это обозначает народ которому понятнее «последовательность шагов» обычно пользуется инструментами специально созданными для отслеживания «последовательных шагов». В данном конкретном случае имелась ввиду жалоба гитотетического вставлятеля, который у нас тут присутствует незримо и все время то забывает во втором кейсе дату логина поставить, то внутренностей реализации не знает. В данном случае он жаловался что не инвертировал условие и два часа сидел в дебаггере. Литературный образ такой, собирательный.

> У тебя дефолтное поведение _пустить_. Это имхо недопустимо для кода связанного с безопасностью.

но прав ты тут только частично — у тебя тоже есть «одноветочные» иф-ы, например if user.inactive => respond(UNAUTHORIZED, «Account is inactive»), и их избежать в принципе невозможно

вопрос в том, как сделать их только необходимыми

> По памяти было что-то вроде: «если вам хочется добавить коментарий - значит вы что-то делаете не так. Лучше выделить часть кода в короткий метод(функцию) с подходящим названием».

не лучше

тем более, что комментарий первичен — при проектировании сверху вниз мысль сначала записывается на человеческом языке, и только затем придумывается функция, которая будет ей соответствовать

> Зачем?

читай всю дискуссию, а не только то, на что отвечаешь

> Это обозначает народ которому понятнее «последовательность шагов» обычно пользуется инструментами специально созданными для отслеживания «последовательных шагов».

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

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

то забывает

о внутренностей реализации не знает.

Н-да-с.

> В смысле что сторадж не имплементит какой нить List ? Угу - не имплементит.

нет — в том, что файнд обычно принимает предикат, а у тебя строку

Тут надо понимать что это не хобби проект на который можно убить сколько угодно времени - потому трудно ожидать что я его отполирую для удовлетворения всем Productа, Seq, Mutable и прочим

надо было кидать туда предикат, и все

> не лучше

тем более, что комментарий первичен — при проектировании сверху вниз мысль сначала записывается на человеческом языке, и только затем придумывается функция, которая будет ей соответствовать

1) Ты подменил обдумывание решения или чего части на запись комментария.

2) Ты подменил написание написание сигнатуры функции + её имения на написание функции полностью (включая тело).

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

> её имения

Следует читать как

её имени

Sorry

>у тебя тоже есть «одноветочные» иф-ы, например if user.inactive => respond(UNAUTHORIZED, «Account is inactive»), и их избежать в принципе невозможно

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

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

Никто не говорит что нельзя написать корректно и так и так. Просто в твоем случае надо зубдаюубедиться что после каждого if стоит return.

Я к стати нарвался именно с внешней схемой автентикации (будь прокляты вендовые одмины - кто ж так делает?) именно с такой логикой: вон там видишь результат exec на ноль проверяется? Это прямо аналогичная логика пустить если «ничего не упало по дороге». А подставили меня настройки домена которые говорили о том что если пароль пустой - логинить под анонимусом. Я этого не знал - случайно нажал энтер без пароля - и обанавелкомхом от smbclient.

>нет — в том, что файнд обычно принимает предикат, а у тебя строку

угу - это «типа» «byPrimaryKey». На самом деле все чуть сложнее в том смысле что это не первичный ключ - в данном случае это просот простой случай. Есть более сложные типа storage.isolate(user.login).find(«some-info»).

надо было кидать туда предикат, и все

Нет. Там precision strike. Константная скорость доступа независимо от количества записей.

Гварды его испугали а for comprehension коотрый легко может сгенерировать MatchError, потому что то как оно работает

Для особо внимательных читателей я еще раз повторю, что список моих претензий к вашему матчингу я неоднократно озвучивал. В последний раз очень подробно здесь. Может вы сможете показать, где там «испуг»?

Мартин Фаулер - не истина в последней инстанции.

Он - фаза которую надо пережить.

Что мне не дает покинуть данный тред, так это бесконечные возможности ловли вас на противоречиях в высказываниях. Как то:

Твой код в блоге - это феерический ужас. Его можно разнести и по фаулеру, и по банде четырех, и по чем угодно.

Не могли бы вы разъяснить, как следует воспринимать Фаулера — как эксперта, чьи рекомендации следует использовать для критики кода, или же «его надо пережить»?

Куда нам до ваших высот

> >Мартин Фаулер с вами категорически не согласен!

Мартин Фаулер - не истина в последней инстанции.

Он - фаза которую надо пережить.

Опять вы блещете пафосными фразами? Лучше бы привели аргументы, почему предложения Фаулера для упрощения восприятия кода неверны

1. куча функций одноразового использования вместо комментариев

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

2. уж если настолько широко все перекроено, что сделаны новые функции, надо было перенсти часть кода в класс АuthScheme (или хотя бы извиниться насчет отсутствия этого)

Что именно? Детали выполнения конкретной аутентификации?

3. ну и длина 50 строк вместо моих 30 — канцелярит во всей красе (я тут не говорю что я написал хорошо, еще порефакторю немного)

Извини, но это не серьезно. Легкость восприятия кода не обязательно должна быть обратно пропорциональна его длине, особенно на такой маленькой разнице в объемах. Если бы это было так, то рулили бы какие-нибудь J и K, а у Haskell-я была бы репутация языка, на котором пишут раздутые до невозможности программы :)

Использование return - полное уродство для функционального языка. В F#, например, подобной конструкции нет, вообще. Там return означает примерно тоже самое, что и в Хаскеле, т.е. эта-функцию. А я считаю, что в Scala следует придерживать стиля общего для функциональных языков, хотя возможностей по legаcy в ней безусловно больше.

К тому же паттерн-матчинг был намного приятнее и понятнее, особенно с использованием Option. Похоже, что ни у кого нет серьезных контр-аргументов :)

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

Это сахар типа do-нотации Хаскелла для использования Either в монадном стиле. Как раз это мне у автора понравилось. Перепиши на if-ах и почувствуй разницу.

Переписал:

  type LoginResult = Either[UserInfo, String]
  def Success[T, E] = Left[T, E] _
  def Failure[T, E] = Right[T, E] _
 
  //@rest.Method(httpMethod = Array(POST))
  def login(request: rest.Request,
      //@rest.Param(name = "login")
      loginParam: String,
      //@rest.Param(name = "password")
      passwordParam: String): Unit =
    doLogin(loginParam, passwordParam) match {
      case Left(user) =>
        request.session("user") = user
      case Right(message) =>
        respond(UNAUTHORIZED, message)
    }
 
  private def doLogin(login: String, password: String): LoginResult = {
    if( null == login ) Failure("You should provide login")
    if( null == password ) Failure("You should provide password")
    AccountsStorage.find(login) match {
      case None => Failure("User not found")
      case Some(user) => handleUserLogin(user, login, password)
    }
  }
 
  private def handleUserLogin(user: UserInfo, login: String, password: String): LoginResult =
    if( user.inactive ) Failure("Account is inactive")
    else if( user.authScheme == "PETRIVKA" )
      handlePetrivkaAuthSchemeLogin(user, password)
    else
      handleUsualAuthSchemeLogin(user, login, password)
 
  private def handlePetrivkaAuthSchemeLogin(user: UserInfo, password: String): LoginResult =
    if( user.passwordMatches(password) ) Success(user)
    else Failure("Authentication failed")
 
  private def handleUsualAuthSchemeLogin(user: UserInfo, login: String, password: String) =
    AccessStorage.access.auth_configs.find(_.key == user.authScheme) match {
      case Some(scheme) =>
        //log.debug("authenticating with " + scheme.command)
        val exec = Runtime.getRuntime.exec(
            scheme.command replace("{login}", login) replace("{password}", password))
        if( exec.waitFor == 0 )
          Success(user)
        else
          Failure("Authentication within " + scheme + " failed")
      case None => Failure("Unknown authentication scheme: " + user.authScheme)
    }
 
  private def respond(code: Int, message:String = "") = {}
 
  private val UNAUTHORIZED = 401

Почему нужно было делать for, в котором многократно вызывались toLeft, и почему if-ы хуже, так до сих пор не понимаю.

> Там return означает примерно тоже самое, что и в Хаскеле, т.е. эта-функцию

Wie, bitte? Вы о чём вообще? О какой «эта-функции» в хаскеле а-ля «return» идёт речь?

http://lamp.epfl.ch/~emir/bqbase/2005/01/20/monad.html

Искать по ключевому слову «eta».

> http://lamp.epfl.ch/~emir/bqbase/2005/01/20/monad.html

Искать по ключевому слову «eta».

Так, и? Что за «эта-функция»? Естественное преобразование? Может речь про eta-conversion из лямбда исчисления? В вообще о чём?