Pull to refresh

Comments 34

Проблемы, которые вижу на первый взгляд:


  1. Реализация может заблокировать все кнопки, не подойдет к задаче, где надо блокировать только одну кнопку
  2. Название функции clickIsLocked не отражает то, что она делает. Вроде как я спрашиваю, заблочен клик или нет, но на самом деле я установаливаю лок.
  3. Подобные блокираторы свидетельствуют о возможной грязной бизнес логике компонентов. Когда совершается некоторое действие через кнопку, которое не предпологает за собой дублирование, то надо блокировать действия пользователя через UI, а не при помощи грязных хаков.

P.S. Велосипед не мой, отправляю как фитбек.

1. Ну как бы да. Она еще ко многим задачам не подойдет. Но вообще это блокировка «дребезга» кнопки. Т.е. для простейших случаев, где надо просто чуть-чуть подождать перед тем как дать возможность кликнусть второй раз. Как я написал в статье, я не смог придумать кейса где надо именно только одну кнопку блокировать. Поэтому в «требованиях» к «новому решению» первым пунктом идет что нет необходимости разделять кнопки.

2. Помоему название вполне подходит, в условии, если клик заблокирован, то выход. Но тут думаю большедело вкуса. Код прям по английски читается: Если клик заблокирован — выход: if (MultiClickFilter.clickIsLocked()) return;

Хотя, интересно было бы узнать ваш вариант названия?

3. Да, они свидетельствуют. Тем не мение, можно рассуждать о сферических идеальных архетектурах, а можно решить трубуемую задачу быстро, просто, и изолировано. И да, это как раз блокирует пользователя через UI. Ну либо я не понял, что значит «блокировать через UI». Поясните?

PS Я понял, что велосипед не ваш, но может быть вы передадите также автору?
  1. Пришлось добавить этот пункт, т.к. хабр перенумеровал мои пункты, начиная с единицы)
  2. clickIsLocked — это утверждение. Возвращающие логику методы можно называть вопросом (например isClickLocked). Но оба эти варианта предполагают, что внутри не происходит блокировка, а просто запрашивается текущее состояние. Мне кажется именно это имел в виду автор комментария.
  3. Блокировать через UI, значит поменять состояние и внешний вид интерфейса так, чтобы пользователь не только не мог нажать то, что не нужно, но ещё и видел, что 1) он таки нажал, можно расслабиться и ждать результата и 2) продолжать искать бесполезно, кнопка серая/продавленная/лопнула/спряталась и т.п.
    Это довольно трудно сделать, но это сразу лишит нас приложений с неочевидно себя ведущим интерфейсом, где ты не понимаешь, нажал ты, не нажал, и что вообще происходит и кто тупит.

Вообще про пользователя надо помнить. Его жалко же. Ему этим пользоваться потом.

1. Ну хоть тут нет разночтений :)

2. Ну если читать по английски, получится вопрос:

if (MultiClickFilter.clickIsLocked()) return;

Звучит как вопрос. А по поводу сайд-эффекта, ну и что? Этот эффект инкапсулирован в этом методе. Клиенту он недоступен, так что тут все нормально.

3. Да. Надо так делать если операция долгая. Так же ее надо выносить в бэкграунд и показывать прогресс. Это все да. Но как бы речь изначально о легаси коде. И речь о дребезге, не более. Т.е. конечно так нельзя блокировать на длительное время, которое значимо для человеческой сознательной реакции.

По пункту 2 об именовании хотел бы сказать:


  • В вопросительных предложениях глагол должен находиться в начале предложения. То есть, "click is locked" = "клик однозначно заблокирован", а в то время как "is click locked" = "а заблокирован ли клик?";
  • Наличие инкапсулированной, но при этом неочевидной логики (как то изменение положения статичного поля locked и запуск таймера) может привести к непредвиденному поведению нового кода с использованием этого метода, если за него возьмётся другой разработчик или же Вы сами достаточно надолго его забросите и вернётесь позже.
Это не вопросительное предложение. Это конструкция с if. По английски не говорят if is apple unripe, don't eat it. Говорят if apple is unripe, don't eat it. На мой взгляд код проще читать если он чатается (извиняюсь за тавтологию) как нормальная фраза. Так же, то, что метод начинается не с is, говорит о том, что это не просто геттер. Что-то внутри делается. А так да, по стандарту Java, геттер возвращающий boolean должен начинаться с is. Но это не просто геттер.

Ну да ладно. Я же не спорю, что может быть можно получше назвать. Так кто-нибудь может скажет лучшее называние? А то как-то немного не корретная дискуссия выходит :(

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

Метод предельно просто. Что бы «вспонить» что он делает, достаточно один раз взглянуть внутрь. Если работаешь с незнакомым (или хорошо забытым) кодом, все равно надо смотреть что внутри.
UFO just landed and posted this here
Ну так она и объясняет. Тринарный оператор это синтаксический сахар. На него не следует ориентироваться. Это как сокращения в естественном языке. Приемлемы, но только в определнных местах и контексте. О какой конкретно конвенции идет речь? Я серьезно? Про геттеры знаю. Про произвольные функции вроде ничего такого не помню.

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

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

Как правильно назвать функцию в данном случае?
UFO just landed and posted this here
Ну да, состояние в конкретный момент вызова этой функции. Ну и что что меняется? Вы когда вызываете любую функцию, она возвращает занчение в конкреный момент. И это значение может меняться.

Например, прям вот самые кишки андроида: BluetoothAdapter.getDefaultAdapter().isEnabled()

Возварщает занчение логическое, в конкретный момент. Через долю секунды это значение может поменяться в результате работы другого приложения или дейсвий пользователя или еще чего. И повторный вызов вернет другое значение вас тоже не устраивает?

Что значит «нелогично с точки зрения языка»? У языка нет т.з. Есть синтаксис. А как им выражать то, что требуется дело программиста.

То вы про какие-то конвенции, которые не можите привести, то про про какое-то требование к постоянству возвращаемого результата… логика языка какая-то…

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

PS: Функция возвращает не предыдущее значение. Она возращает значение в зависимости от прошедшего времени. Что и требуется в рамках задачи.
UFO just landed and posted this here
Вы понимаете, что эта функция сама это состояние не меняет?

Конечно. Какая разница, кто меняет состояние? Если бы это состояние влияло бы еще на что-то, или могло меняться откуда-то еще и при этом не сбрасывалось само, то да. Но это все не наш случай.
И, строго говоря, это не функция сбрасывает занчение, а лябда, которая будет вызвана когда-то потом. Так что даже тут вы не совсем правы.

С точки зрения английского языка, потому что конструкция относится к нему.

А, английского :) Я сначала подумал вы про язык Java. Ну с т.з. английского я уже писал. Как раз с т.з. английского языка тут все хорошо. Стейтмент читается прям по грамматике.

Функция всегда устанавливает значение в true и возвращает значение, которое было до этого, т.е. до вызова функции, именно предыдущее. Когда функция возвращает false она гарантированно возвращает устаревшее значение, которое она сама и поменяла. Прошедшее время она вообще никак не проверяет, кстати.

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

Давайте очень просто. Вы функции называете по тому, что они делают, или что они возвращают? Если первое, то функция должна называться lockClick, если второе — wasClickLocked, правда тогда у вас половина функций должна называться void. Если третье — озвучьте.

Ну если очень просто, то я функции называю по смыслу их действий в соотвествии а английски языком, т.к. нахожусь в англоговорящей зоне. В данном случае она «проверяет блокировку и захватывает, если ее нет». Кроме того, я учитываю использование функции. Если ее называть lockClick, то надо инвертировать результат, что либо приводит опять к смени имени либо засоряет код постоянными отрицаниями результата.

Я правильно понимаю, что вы предпочтете везде делать инверсию возвращаемого значения? Андроид Струдия, кстати, на это ругается втроенным инспекшеном.

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

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

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

Все правильно, is — глагол. Ну и кроме того, я вполне допускаю, что можно принимать конвенцию как жесткое правило. Что по поводу инверсии результата? Вполне возможно, что вы так и делаете и вы не имеете права называть методы более подходяще ситуации. У нас нет таких жестких правил. И не надо натягивать свои конвенции на всех. Мое название не противоречит конвенции по вашей ссылке. По крайней мере Java разделу.

PS: Кроме того, предполагается что, мы тут общаемся на равных. Поэтому постарайтесь избегать консрукций в стиле «вам объяснили». Я вам ничего не должен, а вы не в своей шаражке джуниоров пинаете. В особенности по поводу «предпочтений». Давайте без хамства.

Я все еще хотел бы узнать ответ про инверсию возвращаемого значения в случае вашего варианта называния.
UFO just landed and posted this here
Ещё раз повторю: по ссылке черным по белому написано начинаться с глагола, ваш глагол is стоит не в начале.

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

Будем считать, что вы меня убедили.

Под обратной связью я имел ввиду либо другие подходы, но в рамках Java и без библиотек. Например, вариант с наследником OnClickListener мне в голову не пришел. Но он хорош!

Дело не в том, что вы посмели критиковать. Равно как и я имею право не соглашаться с чем-то.

Про дартаньяна и вокруг это вы придумали потому что вашу критику пришлось отстаивать а не сразу вам врот посмотрели?

Ну бывает. Ладно. В любом случае, позиции понятны. Расходимся :)

ЗЫЖ Я действительно согласен, что lockClick лучше.
А не проще ли написать extension функцию для View в которой фильтровать клики наружу?
Или свой экземпляр расширяющий OnClickListener с тэгированием объекта через View.
Насколько язнаю, в Java нет extensions. По крайней мере под андроид. Можно конечно его вручную эмулировать, но зачем?

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

Я не понял, что значит «фильтровать» наружу?

И независимо от того, экстеншен это или нет, надо как-то определять когда начать и когда перестат фильтровать клики.

Не могли бы вы показать свой пример реализации?
Грубо как-то так:
private val LAST_CLICK_TIME_KEY = R.id.last_click_time

fun View.setOnBlockedClickListener(
    blockTime: Duration = 500.toDuration(DurationUnit.MILLISECONDS),
    listener: (View) -> Unit
) {
    this.setOnClickListener { view ->
        val lastClickTime = view.getTag(LAST_CLICK_TIME_KEY) as? Duration
        val currentTime = SystemClock.elapsedRealtime().toDuration(DurationUnit.MILLISECONDS)
        view.setTag(LAST_CLICK_TIME_KEY, currentTime)
        if (lastClickTime == null || currentTime - lastClickTime > blockTime) {
            listener(view)
        }
    }
}

Но это же будет работать только для наследников View? Ну лично я не люблю засовывать в тэги, но это мое личное дело :)

Мою реализацию так же можно использовать при клике на Item в ListView. Я на этом не акцетировал внимание, наверное зря, но в примере использования именно дребезг на айтеме листвью фильтруется.

В вашем случае надо будте делать отдельный экстеншен.
> Но это же будет работать только для наследников View?
Разве View не является корневым классом в иерархии виджетов Android?

> Мою реализацию так же можно использовать при клике на Item в ListView.
Можно ли по-подробнее, что Вы подразумеваете под Item?
Да, View корень иерархии виджетов. Но тут я имел ввиду ListView. И под Item a подразумевал item из ListView, на котором произошел клик. Т.е. OnItemClickListener.

Я не увеерн, наверное можно будет привязаться к инстансу самого ListView или к View айтема… Но это опять же дополнительный код, если я правильно понял.

Зачем так сложно и непонятно… Сохраняешь в статик переменную время последнего клика. При следующем клике смотришь прошло твоих 500 миллисекунд или нет. Если прошло, то обновляешь время последнего клика. Не забудь завернуть в синглотон. Done.
И да, делается простой и удобный экстеншн метод на котлине в замену обычного setOnClickListener.

Что именно сложно? И что именно непонятно?
Используется один метод. Именно статик переменная там и используется.
Зачем обновлять время и сравнивать, если можно обойтись без этого?

А синглтон то тут причем вообще!?

Еще раз, это все на Java. Java не имеет экстешенов. Давайте придерживаться темы и заданных условий.
> Java. Java не имеет экстешенов. Давайте придерживаться темы и заданных условий.
Наследование?
class OnBlockedClickListener extends OnClickListener {
    @Override
    public void onClick(View v) {
       // Творим что хотим
   }
}
Ну да. Это примерно тоже самое, что и в вашем примере с экстеншеном. В принципе можно и так, но наследование ради этого, ИМХО лишнее.

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

В любом случае это таже вкусовщина. Кто-то любит наследование, кому-то проще без него. Это принципиально ничего не меняет и не делает решение более компактным и изолированным.
uiHandler.postDelayed(() -> locked = false, lockTimeMs);


Не забывайте терминейтить Handler при Lifecycle.destroy ивенте.
Не понял, зачем его терминейтить? Это один нстанс в статическом поле. Он используется для всех вызовов. Насколько я понмаю, моя реализация не имеет никакого отношения к Lifecycle.
для случаев старта другой activity при кликах можно обернуть OnClickListener проверкой

class ResumedClickListener extends OnClickListener {

    private final LifecycleOwner lifecycleOwner;

    public ResumedClickListener(LifecycleOwner lifecycleOwner) {
        this.lifecycleOwner = lifecycleOwner;
    }

    @Override
    public void onClick(View v) {
        if (lifecycleOwner.getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.RESUMED)) {
            // code
        }    
    }
}


или kotlin
inline fun LifecycleOwner.runIfResumed(block: () -> Unit) {
    if (lifecycle.currentState.isAtLeast(Lifecycle.State.RESUMED)) {
        block.invoke()
    }
}
А зачем вообще примешивать сюда Lifecycle? Нам без разницы, есть ли активити и какая. Может активити уже давно уничтожена. Лямда выполниться в любом случае через заданное время.

Я просто хочу еще раз подчеркнуть, это один из вариантов решения проблемы дребезга (debounce), а не попытка дизаблить недоступные по бизнеслогике части интерфейса. Для этого требются совсем другие подходы.
это один из легальных способов кмк избавиться от открытия двух и более activity/fragment/view/dialog в android и очень надежный,
обычно все view находятся внутри чего то что имеет жц, и если клик только про переход в любой другой из activity/fragment/view/dialog, то на текущем месте вызова стейт lifecycle поменяется на paused/stopped и не даст продублировать клик, ваш вариант больше про что то внутри текущего экрана(добавить в корзину товар и тд), тут да велосипеды везде с delay логикой будут.
Ну в приципе согласен. Просто в моем случае жц не используется в приложении совсем. Я об этом как-то не думал. Как выше сказал господин kukovik, по хорошему конечно надо управлять UI, но об этом надо думать на этапе проектирования. Но это не всегда оправдано, поэтому даже в заголовке есть слово «велосипед» :)

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

Ну да, без rx никак не обойтись…

Каких тут телодвижений много? Один if в одну строчку.

О каких вы телодвижениях говорите?

Sign up to leave a comment.

Articles