Как стать автором
Обновить

Комментарии 119

Но и злоупотреблять классами там, где достаточно простого хэша, или структуры — тоже.

Крайне сомнительный совет. Если у вас есть некоторая сущность с вполне себе определенными полями, её практически однозначно нужно выводить в класс, хотя бы ради того, чтобы потом не пытаться понять "а какие именно ключи содержатся в этом хэше в этом месте".

Задача хэша - именно хранить хэш-таблицу "ключ-значение" а не упорядоченную структуру фиксированного типа, поэтому если где-то для хранения экземпляра доменной модели используется хэш, это сразу сигнал того, что вероятно тут где-то откровенный мисдизайн

Ну и логичное следствие, что хранить мапинги типа логин -> User в хэше - нормально и правильно. А за хранить поля юзера в хэш-таблице как в примере - сразу бить по рукам.

P.S.: структура как "чуть более организованый хэш" тоже не слишком хорошая идея.

Ну тут ваше слово против моего. По крайней мере, пока руби не научится деструктурировать инстансы классов в аргументах функции.

Классы не нужны в принципе, а тут — особенно.

рукалицо против вашего слова

Коллега, я вижу, ещё юн — он не был в тех кругах ада, которыми проходил я...

Если вам не нравятся классы, то зачем вы используете язык который основан на концепции ООП?

Разработка не укладывается в бинарную концепцию ООП/ФП (более того, уже лет 20 даже само это разделение — искусственно).

Руби силен метапрограммированием (до появления Эликсира — самый крутой в этом плане язык), гибкостью синтаксиса, изящной адаптацией функционального подхода, лямбдами, наконец. А классы — ну вынужденное зло, Матц на хайпе в 90-е погорячился, с кем не бывает.

В подавляющем числе проектов и по практикам мету не используют и правильно делают. Класс - основа для проектирования и работы самого языка, осмысленные данные такого типа упаковывать в него логично по содержанию. 4 пункт явно очень спорный, отбраковывать кандидатов из-за немного другого подхода, который вам не нравится, но не является ошибкой ни в коем случае - самодурство.

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

Здесь есть ещё другая опасность: опечатки. Например, человек написпал h[:iteratpr] = some_value, а потом удивляется, почему в соседнем методе h[:iterator] возвращаетnil, хотя "вот же оно, я ж его только что присвоил!!!" В случае класса оно сразу заорёт, что "нет такого метода".

Человек не может захотеть вызвать Hash#[]= по очевидным причинам (это метод, а я вроде ведь почти русским по белому написал: «структура данных»).

Да, если нужны вызовы методов на инстансах, придется терпеть класс.

В руби "класссы" — это чуть менее, чем всё. Поэтому давайте определение, что Вы понимаете под словом "структура данных".

Ну ок, я имел в виду, как, мне кажется, довольно понятно из контекста, структурированные данные. «Структура данных» — формулировка неаккуратная, согласен, но первым в тексте встречается именно определение «структурированные данные».

Так почему Hash лучше, чем Struct? Кстати, теперь есть и более удобный Data.

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

Появление Data — оно же ровно для этого.

Но ведь речь была о структурированных данных. И Struct даже посильнее об этом кричит. А Hash скорее кричит о том, что он просто набор каких угодно данных.

Ну я ведь структуры тоже предлагал.

Думаю, что здесь автор имеет в виду какой-то конкретный пример, в котором данный совет очевидно полезен.

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

Да правило проще некуда: если нет действий, которые умеет выполнять только инстанс, а есть просто набор данных — класс не нужен.

Согласен, что лучше классы, чем хэши. Но ещё лучше же Struct/Data, если это чисто данные или обьект-значение.

Класс — это заявка на «я самостоятельная сущность». Данные таковой не являются.

А что неправильного в объявлении переменной вне скоупа и изменении её? Вы внутрь reduce заглядывали, там разве не так?

Да, внутрь Enumerable#reduce я заглядывал (и даже где-то рядом патчи слал). Там вот так:

static VALUE
enum_inject(int argc, VALUE *argv, VALUE obj)
{
    struct MEMO *memo;
    VALUE init, op;
    rb_block_call_func *iter = inject_i;
    ID id;

    switch (rb_scan_args(argc, argv, "02", &init, &op)) {
      case 0:
        init = Qundef;
        break;
      case 1:
        if (rb_block_given_p()) {
            break;
        }
        id = rb_check_id(&init);
        op = id ? ID2SYM(id) : init;
        init = Qundef;
        iter = inject_op_i;
        break;
      case 2:
        if (rb_block_given_p()) {
            rb_warning("given block not used");
        }
        id = rb_check_id(&op);
        if (id) op = ID2SYM(id);
        iter = inject_op_i;
        break;
    }

    if (iter == inject_op_i &&
        SYMBOL_P(op) &&
        RB_TYPE_P(obj, T_ARRAY) &&
        rb_method_basic_definition_p(CLASS_OF(obj), id_each)) {
        return ary_inject_op(obj, init, op);
    }

    memo = MEMO_NEW(init, Qnil, op);
    rb_block_call(obj, id_each, 0, 0, iter, (VALUE)memo);
    if (memo->v1 == Qundef) return Qnil;
    return memo->v1;
}

Ну так и memo вне скоупа объявлено… Что неправильного то в этом, был вопрос.

Во-первых, memo нигде вообще не объявлено. Во-вторых, неправильно в этом то, что такой код становится контекстно-зависим, и при рефакторинге его очень легко поломать.

def fun array, sum = 0
  # код
  
  sum = 0 # вот эту строчку можно удалить, и сразу ничего не сломается 
  array.each { |e| sum += e }
  sum
end

С правильным вариантом на эти грабли не наступить.

Struct тормознутее обычных классов, насколько я помню, хотя не знаю насколько это важно (вот OpenStruct уже не рекомендуют из-за динамического добавления полей тормозов еще больше). А зачем 0 в reduce указывать он же там по умолчанию?

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

Ну, ноль там для большей «обобщенности» примера :)

Возможно я неправильно написал бенчмарк и он показывает неправильные результаты

require 'rubygems'
require 'benchmark'


class Test
  def initialize(a, b)
    @a = a
    @b = b
  end

  def a
    @a
  end

  def b
    @b
  end
end

Test1 = Struct.new(:a, :b)
cycles = 100_000_000

Benchmark.bm do |x|
  x.report("Class") do
    cycles.times do |t|
      cl = Test.new(t, t+1)
      result = cl.a + cl.b
    end
  end

  x.report("Struct") do
    cycles.times do |t|
      st = Test1.new(t, t+1)
      result = st.a + st.b
    end
  end
end

У меня вышло так:

       user     system      total        real
Class 26.309904   0.098226  26.408130 ( 26.427825)
Struct 25.518613   0.055952  25.574565 ( 25.576957)

Разница составила 3% по времени на таком размере операций, т.е. где-то 3.0e-10 секунды на операцию.

Вообще, согласно документации, Struct создает тоже класс

The Struct class generates new subclasses that hold a set of members and their values. For each member a reader and writer method is created similar to Module#attr_accessor.

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

Struct создает тоже класс

Разумеется, мы же в руби. Даже обычный Hash — это класс. И что?

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

Ложное утверждение

Между ними нет разницы. Struct == Class просто с обвязкой для генерации метода initialize и ридеров и врайтеров.

Test = Struct.new(:a) do
  def method_missing(method)
    puts "Missed method #{method}"
    42
  end
end

x = Test.new(10)
x.some_method_what_does_not_exists

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

Код же доступен: https://ruby-doc.org/core-3.1.1/Struct.html#method-c-new

    …
    rest = rb_ident_hash_new();
    RBASIC_CLEAR_CLASS(rest);
    OBJ_WB_UNPROTECT(rest);
    tbl = RHASH_TBL_RAW(rest);
    for (i=0; i<argc; i++) {
        VALUE mem = rb_to_symbol(argv[i]);
        if (rb_is_attrset_sym(mem)) {
            rb_raise(rb_eArgError, "invalid struct member: %"PRIsVALUE, mem);
        }
        if (st_insert(tbl, mem, Qtrue)) {
            rb_raise(rb_eArgError, "duplicate member: %"PRIsVALUE, mem);
        }
    }
    rest = rb_hash_keys(rest);
    st_clear(tbl);
    RBASIC_CLEAR_CLASS(rest);
    OBJ_FREEZE_RAW(rest);

Мало того, от Struct можно еще и наследоваться.

Ну если такая разница становится принципиальной, то, по-моему, стоит сменить язык. Struct всё же нужен скорее для большей строгости.

[1, 2, 3, 4].reduce(0, &:+)

Это явно хуже читается, чем предыдущий, "плохой", вариант. А код, в первую очередь, должен хорошо читаться и пониматься, при прочих равных. Компьютеру все равно, как выглядит код, а вот человеку - нет.

Вот за это мне и не нравится современный руби. Начали тащить синтаксис отовсюду и добавлять закорючки в язык. И главное, зачем, лень несколько лишних кнопок нажать что-ли. Так все равно же авто дополнение есть. А читабельность падает.

Это синтаксис притащен из перла в версии 0.1 (1993 год). И он отлично читается, если хотя бы на уровне джуна уметь программировать. А тут речь про целого синьёра.

Вот эта закорючка "&:+", к примеру, мной отлично не читается и требует некоторого напряжения и времени, чтобы я её осознал. А если просматривать большой код, то 100% эта закорючка пройдёт мимо внимания. А если в этом коде таких закорючка будет много, то глаза в кучку гарантированы.

Справедливости ради &:method это стандартная и довольно часто используемая конструкция рубей.

Вы небось на this::methodв Джаве тоже ругаетесь?

В данном варианте можно вообще Array#sum использовать

[1, 2, 3, 4].sum

Спасибо, буду знать.

№3 Hash.default_proc

Это любимая ахиллесова пята всех без исключения рельсовиков (стр. 7), ||:

Крайне вредный совет, на мой взгляд, в результате мы не видим что вернется в случае nil на месте, а должны бегать по проекту и искать где этот хеш был определен и смотреть где определена его default_proc, а еще она может быть переопределена или состоять из километровой портянки switch(key)

О, да! А 10 разных рассинхронизированных дефолтов, разбросанных по коду, — гораздо лучше использования стандартных продуманных средств языка.

Вот простой код, представим что классы разбросаны по проекту. Что мы получим в результате?
На первый взгляд 10 и "a"

some_hash = {a: 1, b: 2}

class A
  def initialize(hash)
    @hash = hash
    @hash.default_proc = proc { |hash, key| 10 }
  end

  def hash
    @hash
  end
end

class B < A
  def initialize(hash)
    super
    @hash.default_proc = proc { |hash, key| "a" }
  end
end

a = A.new(some_hash)
b = B.new(some_hash)

puts a.hash[:empty]
puts b.hash[:empty]

А если поменяем местами

a = A.new(some_hash)
b = B.new(some_hash)

Категоричность суждений, в данном случае, ведет к радостям отладки.

О да!!

Почему-то люди постоянно забывают, что "передача объекта" — это под капотом на самом деле передача указателя на объект — а если в вызываемом методе начать менять содержимое объекта, то там можно таких сюрпризов для вызывающего натворить...

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

Если говорить только про default_proc, здесь сразу три проблемы:

  • её никто никогда не устанавливает из методов на инстансе

  • её никто никогда не устанавливает перезатирая существующее значение (стратегии мерджа надо обсуждать по месту)

  • её никто никогда не устанавливает на объектах, не принадлежащих callee

Из моего опыта собеседований довольно много кто зазубрил всякие вещи и пишет очень метапрограммный код, но допускает в простейших вещах с AR такие перлы:

def update
  @counter = Counter.find params[:id]
  @counter.clicks += 1
  @counter.save
end

А когда спрашиваешь где тут может быть проблема пишут код типа такого:

def update
  Counter.transaction do 
    @counter = Counter.find params[:id]
    @counter.clicks += 1
    @counter.save
  end
end

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

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

Скорее всего, дело в разнице профилей: я не пишу прикладной код, я пишу библиотеки, при помощи которых команда пишет прикладной код. (Оговорюсь сразу: я отвечал метапрограммированием на вопрос «почему я выбрал руби», и это не имеет никакого отношения к оригинальной теме собеседований: я не жду от соискателей никакого метапрограммирования).

Так вот, имею сообщить: AR — это примерно на 102% метапрограммирование (среднего качества, да, вот вы выше прекрасный пример привели, почему именно). И то, что вы задаете такие вопросы на собеседованиях — прямое следствие сильнейших возможностей языка в этом плане, без них просто никаких рельсов бы не было.

Кроме того, это ложная дихотомия. Можно увидеть гонку в коде выше и все еще хорошо владеть метарпограммированием.

допускает в простейших вещах с AR такие перлы

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

Проблема №1

Судя по тому, что идёт обращение к params, этот код должен находиться в контроллере, потому как именно ApplicationController (или ActionController::Base) определяет params. Внутри модели этот код без дополнительных телодвижений работать не будет.

Проблема №2

Нет никакой валидации входных параметров. Что если нам передали хэш, в котором нет ключа :id?

Проблема №3

Нет обработки аварийных ситуаций. Если Counter с таким id не существует, код упадёт. Это некузяво.

Проблема №4

Использовать переменную экземпляра ( @counter), если в этом нет необходимости, некузяво. Вместо этого лучше было использовать локальную переменную counter.

Проблема №5

Имеет место race condition. Мы уже установили, что код вызывается из контроллера — следовательно, мы имеем дело с сервером, который обслуживает многих юзеров. Если более одного юзера привели к вызову метода update с одним и тем же id в одно и то же время, и при этом таблица counters была создана без колонки lock_version, то возможна ситуация, когда clicks увеличится только на 1 (вместо ожидаемого 2), а если c колонкой lock_version, то код упадёт по Stale Object

Проблема №6

Обёртка в транзакцию проблемы race condition не решает. Транзакция гарантирует, что группа операций записи либо целиком выпонится, либо целиком не выполнится. Однако здесь операция записи в базу всего одна — так что обёрнуто ли оно в транзакцию, не обёрнуто ли — ничего не изменится.

Чтобы оно работало нормально,

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

def update
  Counter.where(id: params[:id]).update_all("clicks = ISNULL(clicks, 0) + 1")
end

Все по делу, но Counter.where(id: params[:id]) — это для читателя такого кода прям гигантский WTF :)

А также: использовать AR, не гнушаясь писать бд-зависимый код, — дурной тон.

IF работает в любом RDBMS из мне известных, и не намного прям длиннее.

Во-первых, смысл AR именно в том, чтобы максимально скрыть зависимости от конкретной базы.

Во-вторых, движение в сторону использования AR-обёрток .where(...) вместо чистого SQL (clicks = (IF clicks IS NULL THEN 0 ELSE clicks END) + 1) идёт уже не менее чем пяток лет. Следим за тенденциями-с!

В-третьих, положа руку на сердце: за вот уже 10 лет существования проекта мы поменяли движок базы ровно 0 (ноль) раз. Зачем закладываться на бд-независимый код, если скрипач по факту не нужен?

смысл AR именно в том, чтобы максимально скрыть зависимости от конкретной базы

Спасибо, буду знать.

движение в сторону использования AR-обёрток

Я про ISNULL которого за пределами MSSQL не существует.

скрипач по факту не нужен

Ну так я и сказал: «дурной тон», а не «ловите джуна» ведь :)) Не нужен, не нужен, тут согласен.

Спасибо, буду знать.

Сарказм детектед. Я знаю, что Вы знаете. Но Вы забываете первое правило споров в интернете: мы с Вами тут не одни, на нас люди смотрят. Соответственно, я текст пишу не столько для лично Вас, сколько для зевак — которые как раз могут и не знать.

Да ладно, уж прям не саркастнуть к месту. Я же по-доброму.

А когда шутят в курилке, громко объявляют: «Внимание, шутка!»?

Я не стремлюсь всем понравиться, а уж тем, кто на беззлобные подколки обижается — и подавно. Расставлять дебильные смайлики и закрывать предложения скобочками, чтобы собеседник не подумал лишнего — это не ко мне, я способен писать внятно без суржика.

Мне тут отсутствие отработки ошибок (ну и save! вместо save) в голову пришло, ну или update поля, переменная вместо переменной класса - лучше, но никак не проблема.

переменная вместо переменной класса — лучше, но никак не проблема

Эм. Во-первых, это переменная инстанса (aka instance variable), а не класса.

Во-вторых, это потоконебезопасно.

Если бы тут была не переменная инстанса, а переменная класса — то за такое сразу ссанымми тряпками за сто первый километр.

Я хотел было рассказать байку про переменную класса в первом примере, но это было бы в тексте лишнее, так что расскажу тут.

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

Первый уже не помню какой, что-то в метапрограммировании (кажется, написать логику на отличии __callee__ от __method__, примерно, как git со своим бинарником делает), я его довольно быстро закрыл. Потом я полгода искал удобный случай протащить в прод flip-flop — получилось и это, в парсере чужого мохнатого формата из 90-х. А вот использовать переменную класса с пользой — года три надо мной дамокловым мечом висело.

И вот, была у меня некая система плагинов, в которой было позднее связывание. Voilà — роутинг уехал в @@routing, а я с тех пор на руби почти не пишу :)

А я вот уже лет эдак двадцать как осознал, что я работаю не один, а в команде (где далеко не все — гении), и времена, когда я мог писать идеально работающий код, который никто, кроме меня, понять не может, прошли. В результате родилось two second rule: "если код не может быть понят читающим за две секунды, он должен быть переписан".

К сожалению, все Ваши гештальты это правило не проходят. Так что в личном проекте они имеют право на существование, а в рабочем нафиг с пляжа — извините, you are not a team player, Вы нам не подходите.

Да я и работу-то в принципе не ищу, и с тезисом абсолютно согласен.

Просто у нас процентов 10 продакшн кода — мои личные проекты. OSS. Как `sidekiq`, только фича реквесты быстрее в работу принимаются.

В общую кодовую базу я, разумеется, такое не потащу, даже странно объяснять.

Я что-то привык, что говоря "переменная класса" это ясно что переменная экземпляра класса. Именно переменную класса я видел только один раз в реализации синглтона (хз зачем). А по поводу потокобезопасности, что-то не встречал, чтобы именно в таких случах как-то заботились об этом, это видимо очень редко всплывающая проблема.

Если всего один раз всплывет проблема в транзакции но €40М, этого будет достаточно.

Если всего один раз всплывет проблема в транзакции но €40М, этого будет достаточно.

You must be Brad.

Мне тут отсутствие отработки ошибок (ну и save! вместо save) в голову пришло

Отличие save!в том, что оно вызывает валидаторы и падает, если валидаторы не прошли. Но в данном случае объект достаётся из базы (то есть когда его туда записывали, валидаторы уже прошли — иначе его бы в базе не было), сейчас изменился только счётчик — на что его валидировать-то? В данном конкретном случае save и save! по факту будут работать абсолютно одинаково в 99,99999999% случаев.

Про Stale Object Error я уже сказал ("Проблема №5"), про потенциальную возможность отсутствия записи с таким ID тоже ("Проблема №3"), а других потенциальных ошибок при работе с базой (ну, кроме как "база ВНЕЗАПНО стала недоступна" и т.п., но в таких случаях сервер уже может заворачиваться в белую простыню и далее по тексту) здесь я не вижу — в коде, выполняющем единственное действие, много ошибок совершить и правда сложно.

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

def update
  counter = Counter.find(params[:id])
  counter.with_lock do
    counter.increment!(:clicks)
  end
end

Например потому, что решение выше работает всегда, а ваше — лишь иногда, по крайней мере, в том виде, как вы его представили.

Подсказка: этот метод может быть вызван в конкурентной среде в двух разных потоках одновременно.

почему мое решение не будет работать в конкурентной среде? Используется лок. На уровне БД будет блокировка `SELECT FOR UPDATE`.
Если два одновременно потока/запроса будут выполняться, то один из запросов повесит лок на строку, а второй увидев блокировку, будет ждать снятия блокировки. Как только первый запрос выполнится, он снимет блокировку. И после этого выполнится второй запрос. И это обеспечивается СУБД. Почему это не будет работать в конкурентной среде?

Я знаю, как работает with_lock. В отличие от кода выше, он стартует транзакцию, а ресурсы в любом приложении конечны, к сожалению. И — рано или поздно — вы увидите вот такую неприятную штуку:

Mysql2::Error: Lock wait timeout exceeded; try restarting transaction

Т.е. все-таки получается мы приходим к тому, что локи могут помочь с race condition. Или нет? ))
Т.е. по вашей логике раз мы когда-то гипотетически можем упереть в таймаут по локам, то мы будем сразу отказывать от этого решения, и будем за место этого применять неидеоматичные костыли? отказываться от решения, которое как раз задумано для такого типа задач )

Конкретно в нашем примере еще нужно очень постараться добраться, чтобы начать ловить ошибки по таймату. У нас в примере короткая быстрая транзакция, в которой только один апдейт одной записи. Из своего опыта, словит ошибки таймаута по локу происходит в кейсах, когда в одной транзакции несколько тяжелых операций и там висит лок. А у нас апдейт только одной записи в транзакции. Если посыпались такие ошибки на коротких транзакциях, то это говорит либо, что настройки у БД неоптимальны, либо серьезно возросла нагрузка. Настолько, что уже и без локов постгресу/mysql поплохеет. Но в этом случае нужно уже по-другому решать вопрос.

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

неидеоматичные костыли

ActiveRecord::Relation#update_all в данной ситуации — это идиоматичное правильное решение (а лок — костыль). Первое не позовет колбэки, не станет валидировать все поля (а транзакция — внезапно — станет), и является атомарным апдейтом на уровне базы (о чем сказал предыдущий оратор).

То, что добрая половина рельсов идиоматически предлагает делать абсолютно идиотские вещи — вовсе не означает, что не нужно все делать правильно, хотя бы, когда возможно. Напомню, что идиоматический для рельсов выбор из таблицы один-ко-многим — до четвертой, кажется, версии, — порождал печально известную проблему N+1. Зато идиоматически.

на счет валидации - тут да, она не нужна в данном примере, и это плюс.
Атомарный апдейт - как он поможет с гонками?

таймаут по локам - это как раз превышение времени ожидания снятия лока, который повесила другая транзакция. Т.е. от времени выполнения зависит напрямую.

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

мы когда-то гипотетически можем упереть в таймаут

"Если какая-то неприятность может случиться — она случается" (c)

Почему бы не использовать обычный лок?

Потому что рельсам — рельсово, а базе — базово. Почему за локами, транзакционностью и проч. должен следить rails со своими костылями и велосипедами, если база делает это нативно?

Специально зарегистрировался что бы оставить комментарий)

Почему нельзя чз transaction(isolation: serializable) просто сделать? На постгресе неплохо работает

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

База не резиновая, транзакция — тяжела (в сравнении с инструкцией), валидировать всю запись а потом вызывать все колбэки — лишний оверхед.

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

так БД сама по себе не обеспечивает гарантию предотвращения например lost update, как в нашем примере - при конкурентных запросах могут потеряться клики. Вернее БД может это делать, но нужно переключить уровень изоляции на serializable, но тогда БД будет сильно тормозить при сколько-нибудь существенной нагрузке.
Тоже самое с транзакциями - ты сам должен оборачивать в транзакции последовательность операций, которые нужно выполнить атомарно.

Еще есть метод icrement

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

Так inrement как раз сгенерирует SQL запрос который предлагали выше. Лока в данном случае не будет

А, вы имеете в виду вне лока? Да, можно. Только не increment, который вообще в базу не ходит, и даже не increment!, который просто писал какой-то пьяный медведь, судя по коду, и в котором все еще есть гонка, а кошерный update_counters, который, действительно, выполнит атомарный UPDATE.

increment! использует update_counters и там нет гонки, просто он учитывает все increment которые были до сохранения.

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

Вам еще не надоело спорить на ровном месте? Вот гонка во всей красе.

change = public_send(attribute) - (public_send(:"#{attribute}_in_database") || 0)
self.class.update_counters(id, attribute => change, touch: touch)

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

С чего это вы взяли, что к незнакомым? Сообщество довольно тесное, тут же не джава.

Хорошо, вот код:
object = Counter.where(id: 10).first!

Мы делаем запрос к базе, получаем object.clicks = 0, object.clicks_in_database = 0

Делаем increment, получаем clicks = 1, clicks_in_database = 0
Делаем increment!, получаем clicks = 2, clicks_in_databse = 0, вызываем update_counters с параметрами 10, {:clicks => (2 - 0), touch: true}, который делает запрос с UPDATE ... SET clicks = clicks + 2 ...
Единственная гонка которая может тут быть это из-за touch на тему последнего обновления.

Контроллер 1 достал значение, потом контроллер 2 достал значение, потом контроллер 2 обновил значение, потом контроллер один обновил ... Упс!

Или у вас нагрузка 1 реквест в час?

Контроллер 1 послал значение в increment_counters +1 через increment!
Контроллер 2 послал значение в increment_counters +1 через increment!

Вот только что написал в тестовом проекте такой контроллер и сгенерировал нагрузку через siege.

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

  • Заумно и непонятно - новый человек должен понимать что происходит быстро

  • Размазывая логику - не стоит заставлять бегать по 1000 файлам для понимания того что вернется

  • Напирая до абсолюта на какие-либо принципы (DRY в худшем варианте, например)

  • Используя конструкции и подходы не являющиеся частью языка. (см. пункт 1)

а почему вы взяли, что гонки не будет? Это про решение выше с where и про это? атомарность - это вообще не про это. Атомарность - либо выполнится все, либо ничего. А для гонок - как раз механизм локов, либо оптимистическая блокировка, где разруливать на уровне приложения надо. Ну либо включать serializable в БД, но тогда БД будет еле волочиться.

Потому что я умею читать руби код по ссылке.

Еще раз: один вызов `UPDATE` в базе: либо выполнится, либо нет.

при чем тут руби код? Без локов клики могут потеряться при конкурентных вызовах запросов, даже если написать запросы на чистом sql

Нет, не могут. Пожалуйста, подумайте, это не сложно.

ну вы аргументируйте, а не отвечайте типа сам дурак.
Почитайте про аномалию lost update в БД при конкурентных апдетах. И как ее решать.

Lost update тут вообще ни при чем, он не может произойти на атомарном апдейте.

Вот сиквел, который генерируют рельсы для update_counters:

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

я спрашивал, почему вы считаете, что атомарный апдейт дает гарантию предотвращения lost update?

Во-первых, вы этого никогда не спрашивали.

Во-вторых, вы мне там рекомендовали что-то почитать — ну вот и почитайте. Вам дали отправную точку — копайте, если интересно. Если нет — ну считайте, что вы правы, мне-то что?

до этого вы утверждали, что решение на локах не будет корректно работать в конкурентной среде:

Например потому, что решение выше работает всегда, а ваше — лишь иногда, по крайней мере, в том виде, как вы его представили.

Подсказка: этот метод может быть вызван в конкурентной среде в двух разных потоках одновременно.


так что я думаю, если отбросить эмоции и детское "сам дурак, а я умный", то и для вас эта дискуссия была полезная и вы узнали что-то новое )

Рад, что вы так думаете.

ИЧСХ, на заданный мной вопрос "А что конкретно Вам тут не нравится?" так до сих пор никто и не ответил.

И да, решение на локах работает не всегда, даже перечитывание треда поможет это при желании понять.

Транзакции — вообще жуткий костыль, который сначала приводит к lost updates, а потом мы с ним героически сражаемся локами, которые — еще больший костыль.

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

И да, решение на локах работает не всегда

А я так сразу и написал — "в завимости от того, чего Вы хотели, могут потребоваться те или иные решения". Есть где развернуться.

 и лет пять не пишу на руби. Перерос.

А статья тогда это фантомная боль?

Я вот лет 10 не пишу на РНР, но что-то меня не тянет смотреть РНР код и говорить "вы все делаете неправильно"

Я не говорю «вы все делаете неправильно», я дал несколько советов людям, способным учиться.

Я не виноват в том, что в индустрии так много некомпетентных людей, мнящих себя синьёрами.

А еще я продолжаю поддерживать свои OSS библиотеки и (как написано в первом предложении) проверяю тестовые задания в нашей компании.

Зачем тогда вообще статью писать? Чисто чсв потешить?

Не очень понял вопрос. Какая связь между возможностью поделиться с аудиторией и желанием объяснять прописные истины по триста раз каждому комментатору?

А такая, что твои истины прописные вовсе и не истины, о чем тебе триста раз сказали?) Где вообще можно набраться такой уровень уверенности в себе?)

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

Также крайне маловероятно, чтобы мы пили на брудершафт (для этого вам бы не удалось пройти интеллектуальный ценз), поэтому «ты» — неуместно.

Начал читать с мыслью, что узнаю как не опозориться на собесе и узнал, всего то надо не вести себя как автор статьи. А именно грубить, чувствовать себе экспертом который перерос руби и не пишет на нем 5 лет, но считает себя лучше синьоров которые пишут и читают тысячи строк рубишного кода в неделю. Использование **kwargs вместо harg = {}, и это молчу что и то и другое является антипатерном и лучше явно указывать что принимает метод, экспоуз всей внутренней структуры только потому что лень написать h.fetch(:key, "default_value") и это только по одному примеру.

При этом, я уже лет 20 в глаза не видел сиквел

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

CASE COUNT(DISTINCT(?->>'currency'))
WHEN 0 THEN JSON_BUILD_OBJECT('currency', NULL, 'amount', 0)
WHEN 1 THEN JSON_BUILD_OBJECT('currency', MAX(?->>'currency'), 'amount', SUM((?->>'amount')::int))
ELSE NULL
END

Почему не подходит fetch, я тут раза три в комментариях рассказал. (**kwargs) — устоявшийся в коммьюнити способ показать в примере, что принимает функция, без дополнительных комментариев (антипаттерн, лолшта).

Можно читать миллион строк кода в день, и все равно не понимать очевидных вещей.

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

Почему не подходит fetch, я тут раза три в комментариях рассказал.

Не вижу где, по слову fetch на странице только 3 комментария, включая этот.

устоявшийся в коммьюнити способ

Комьюнити elixir? Вот им и расказывается что и как не далать, а отойти от разработки на руби 5 лет, а потом начать всех учить как не стоит писать на руби - смешно.

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

Просто ткнул вас носом в ваше вранье про 20 лет без сиквела, так тоже бывает и, по все видимости, довольно часто.

по слову fetch

Hash#fetch без блока делает (исходники же доступны, почему так сложно туда заглянуть?) ровно то же самое, что ||

if (hash_stlike_lookup(hash, key, &val)) {
    return (VALUE)val;
}
else {
    if (block_given) {
        return rb_yield(key);
    }
    else if (argc == 1) {
        VALUE desc = rb_protect(rb_inspect, key, 0);
        if (NIL_P(desc)) {
            desc = rb_any_to_s(key);
        }
        desc = rb_str_ellipsize(desc, 65);
        rb_key_err_raise(rb_sprintf("key not found: %"PRIsVALUE, desc), hash, key);
    }
    else {
        return argv[1]; // СМОТРЕТЬ ВОТ СЮДА
    }
}

Впрочем, мне не сложно повторить: fetch определяет дефолтное значение в месте вызова, что не лезет ни в какие рамки: тут 5, тут — :ok, а вон там — вообще забыли.

Комьюнити elixir?

Нет, руби.

Вот им и расказывается что и как не далать

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

а отойти от разработки на руби 5 лет, а потом начать всех учить как не стоит писать на руби - смешно.

Да нет, как показывают буквально все комментарии под этий заметкой, — кроме Wesha тут нет ни одного человека, который мог бы хотя бы на лычку «мидл» претендовать.

ткнул вас носом в ваше вранье про 20 лет без сиквела

Чего? Вы там берега не попутали? Если вы что-то куда-то и ткнули — так это только себя носом в дерьмо.

Hash#fetch без блока делает (исходники же доступны, почему так сложно туда заглянуть?) ровно то же самое, что ||

совсем не то же самое, какой смысл скидывать исходники если вы их сами не читаете или не умеете читать, fetch отработает только в том случае если в хэше не найден ключ, в то время как || отработает когда lookup вернул falsy value, если для вас этот одно и то же, то почитайте учебники по логике, параграф про тождество.

Впрочем, мне не сложно повторить: fetch определяет дефолтное значение в месте вызова, что не лезет ни в какие рамки: тут 5, тут — :ok, а вон там — вообще забыли.

Так в том и дело, мы явно указываем, что получить и когда, так как значения могут быть разных типов, строки, числа, массивы и в этом случае нужно городить в default_proc кучу условий, проверяя какой ключ вызван и подставлять дефолтное значение, я уже молчу о том что fetch позволяет строго подходить к lookup-ам и рейзить ошибку, что бывает очень полезно в процессе разработки, исключая возможность опечаток.

который мог бы хотя бы на лычку «мидл» претендовать

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

Ух, ясновидящий в чате.

Я не провожу собеседования, для этого есть эйчары.

А где это вы посмотрели в мой код, интересно? Я, вроде, ссылок не давал.

А где это вы посмотрели в мой код, интересно?

Например, тут.

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

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

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

вытащив из глаз затычки, предотвращающие вытекание

Не подскажите где брали? Я бы прикупил /s

От вас — звучит как комплимент.

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

Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Публикации

Истории