Я в питоне новичек. Этап написания Hello World успешно пройден, двигаюсь дальше
Сейчас делаю ревизию своих поделок на наличие в них не pythonic кода. По результатам этой работы придумалось решение одной типовой задачки.
Задачка: есть список клиентов. Добавлять очередного в список только если в нем нет клиента с таким же именем. Если есть, то кидать исключение.
Решение в лоб:
class Customer:
def __init__(name, phone):
self.name = name
self.phone = phone
_cusomers = []
def add_customer(c):
for e in _cusomers:
if e.name == c.name:
raise ValueError
_cusomers += c
Функция add_customer() ужасна своим циклом for. Крутить цикл только чтобы кинуть исключение — совсем не pythonic.
Так стало получше:
def add_customer(c):
if filter(lambda e: e.name == c.name, _cusomers):
raise ValueError
_cusomers += c
Избавился от цикла, но функция стала делать лишнюю работу:
— всегда перебираются все элементы _customers;
— создается список только чтобы проверить что он не пустой;
А потом придумалась такая загогулина:
def add_customer(c):
def test_customer_exist(e):
if e.name == c.name:
raise ValueError
filter(test_customer_exist, _cusomers)
_cusomers += c
От for избавился. Никакой лишней работы не далается. Одна беда — букв много из-за вложенной функции. Эх, если бы raise можно было запихнуть в лямбду...
Как вам?
Здравствуйте, alsemm, Вы писали:
A>Задачка: есть список клиентов. Добавлять очередного в список только если в нем нет клиента с таким же именем. Если есть, то кидать исключение.
Здравствуйте, alsemm, Вы писали:
A>Я в питоне новичек. Этап написания Hello World успешно пройден, двигаюсь дальше
1 пункт, алгоритмический. Используй cловарь names -> customers
2 пункт, сравнительный. Первый способ по моему лучше остальных.
3 пункт, идиотский. Чтобы использовать raise в лямбде заведи себе функцию throw, которая, правда, будет добавлять к стектрейсу ещё и свой вызов.
Зато тогда, чисто для поржать ты сможешь зачем-то писать вот так:
Здравствуйте, alsemm, Вы писали:
A>Я в питоне новичек. Этап написания Hello World успешно пройден, двигаюсь дальше
A>Сейчас делаю ревизию своих поделок на наличие в них не pythonic кода. По результатам этой работы придумалось решение одной типовой задачки.
A>Задачка: есть список клиентов. Добавлять очередного в список только если в нем нет клиента с таким же именем. Если есть, то кидать исключение.
1. Действительно использовать dict или set. Конечно, если не важна последовательность клиентов.
2. Мне больше по душе Ваш второй вариант, разве что синтаксически немного другой:
def add_customer(c):
if [x for x in _customers if x.name == c.name]:
raise ValueError
_cusomers += c
3. В тех местах, где _действительно_ нужно оптимизировать такого рода поиски я использую генераторы:
def add_customer(c):
for aux in (x for x in _customers if x.name == c.name):
raise ValueError
_cusomers += c
Здравствуйте, alsemm, Вы писали:
A>def add_customer(c): A> for e in _cusomers: A> if e.name == c.name: A> raise ValueError A> _cusomers += c A>Функция add_customer() ужасна своим циклом for. Крутить цикл только чтобы кинуть исключение — совсем не pythonic.
Чтобы "поржать" по-питоновски, можно было сделать так:
def add_customer(c):
if c.name not in [x.name for x in _customers]:
_customers.append(c)
Кстати, для оформления кода используй тег <python> (угловые скобки заменить на квадратные).
Но вообще тебе правильный совет давно дали — словарь по имени.
Здравствуйте, SergH, Вы писали:
SH>Здравствуйте, alsemm, Вы писали:
A>>Задачка: есть список клиентов. Добавлять очередного в список только если в нем нет клиента с таким же именем. Если есть, то кидать исключение.
SH>А, эээ, тупо использовать словарь, а не список?
Ок, запихнул всех клиентов в словарь. Потом возникла необходимость добавлять клиентов с уникальными телефонами. Делать еще один словарь?
Здравствуйте, lomeo, Вы писали:
L>3 пункт, идиотский. Чтобы использовать raise в лямбде заведи себе функцию throw, которая, правда, будет добавлять к стектрейсу ещё и свой вызов.
А что такого в доп. элемент в стектрейсе?
L>Зато тогда, чисто для поржать ты сможешь зачем-то писать вот так:
L>
Здравствуйте, HiSH, Вы писали:
A>>Задачка: есть список клиентов. Добавлять очередного в список только если в нем нет клиента с таким же именем. Если есть, то кидать исключение.
HSH>1. Действительно использовать dict или set. Конечно, если не важна последовательность клиентов. HSH>2. Мне больше по душе Ваш второй вариант, разве что синтаксически немного другой: HSH>
HSH>def add_customer(c):
HSH> if [x for x in _customers if x.name == c.name]:
HSH> raise ValueError
HSH> _cusomers += c
HSH>
HSH>3. В тех местах, где _действительно_ нужно оптимизировать такого рода поиски я использую генераторы: HSH>
HSH>def add_customer(c):
HSH> for aux in (x for x in _customers if x.name == c.name):
HSH> raise ValueError
HSH> _cusomers += c
HSH>
Мне варианты с for — не нравятся . Воспринимаются они хуже imho
Здравствуйте, alsemm, Вы писали:
A>Я в питоне новичек. Этап написания Hello World успешно пройден, двигаюсь дальше
А я в Питоне вообще никакой, поэтому на JavaScript:
// эта функция делает multimap.
// Элементы с одинаковым key собираются в array function add(map, key, val)
{
var pv = map[key];
if( pv === undefined ) { map[key] = val; val[0] = val; } // brand new keyelse if( typeof pv == "array" ) pv.push(val); // more than one value under the keyelse if( typeof pv == "object" ) { map[key] = [pv,val]; pv[0] = undefined; } // mutation of single value into arrayelse
alert( "unsupported type" );
}
Концептуально: тебе нужно использовать и имена и телефоны, т.е.
В этом случае по customers ты эффективно можешь получать следующие выборки:
1) все кастомеры на телефоне NNN-NNNN
2) все телефоны кастомера AAAAAAA
3) есть ли кастомер с телефоном NNN-NNNN и именем AAAAAAA
Здравствуйте, c-smile, Вы писали:
CS>Здравствуйте, alsemm, Вы писали:
A>>Я в питоне новичек. Этап написания Hello World успешно пройден, двигаюсь дальше
CS>А я в Питоне вообще никакой, поэтому на JavaScript:
Свалить всю в один map — это конечно хорошо, пока в системе не появится клиент с именем NNN-NNNN или/и телефоном AAAAAA
Здравствуйте, alsemm, Вы писали:
SH>>А, эээ, тупо использовать словарь, а не список? A>Ок, запихнул всех клиентов в словарь. Потом возникла необходимость добавлять клиентов с уникальными телефонами. Делать еще один словарь?
Не надо словарь, достаточно одного множества (которое set()). Переопредели у Customer метод __hash__() , где считай хэш от всех нужных полей сразу и смело запихивай объекты кастомеров в это множество — дублей не будет. Как реализованы хэши для питоновских строк и целых чисел можно почитать здесь, в качестве примера.
Здравствуйте, alsemm, Вы писали:
A>Здравствуйте, c-smile, Вы писали:
CS>>Здравствуйте, alsemm, Вы писали:
A>>>Я в питоне новичек. Этап написания Hello World успешно пройден, двигаюсь дальше
CS>>А я в Питоне вообще никакой, поэтому на JavaScript:
A>Свалить всю в один map — это конечно хорошо, пока в системе не появится клиент с именем NNN-NNNN или/и телефоном AAAAAA
А какие проблемы при этом могут возникнуть?
Зато потом неважно как искать. Т.е. можно иметь search field единый.
Здравствуйте, alsemm, Вы писали:
A>Я в питоне новичек. Этап написания Hello World успешно пройден, двигаюсь дальше A>Сейчас делаю ревизию своих поделок на наличие в них не pythonic кода. По результатам этой работы придумалось решение одной типовой задачки. A>От for избавился. Никакой лишней работы не далается. Одна беда — букв много из-за вложенной функции. Эх, если бы raise можно было запихнуть в лямбду... A>Как вам?
Плохо. В первую очередь доджно быть не pythonic, а читабельно.
P.S. IMHO все варианты в этой ветке менее понятны для беглого взгляда чем изначальный вариант.
Здравствуйте, Константин, Вы писали:
A>>Я в питоне новичек. Этап написания Hello World успешно пройден, двигаюсь дальше :) A>>Сейчас делаю ревизию своих поделок на наличие в них не pythonic кода. По результатам этой работы придумалось решение одной типовой задачки. A>>От for избавился. Никакой лишней работы не далается. Одна беда — букв много из-за вложенной функции. Эх, если бы raise можно было запихнуть в лямбду... A>>Как вам?
К>Плохо. В первую очередь доджно быть не pythonic, а читабельно.
К>P.S. IMHO все варианты в этой ветке менее понятны для беглого взгляда чем изначальный вариант.
Очень странный подход. Вместо того, чтобы пользоваться вариантами, в которых чётко сказано, что делается — ты хочешь вариант, в котором написано, как оно делается (проходом по циклу), после чего ты:
1) разбираешь код этого варианта, выделяешь цикл и действие в нём;
2) логически "сворачиваешь" этот цикл, выделяя, что же собственно планировалось сделать (а именно — найти клиента с тем же именем);
3) идёшь опять "вглубь" проверяя код на соответствие этому замыслу (нет ли плюх реализации).
После чего эта информация — что именно делалось — теряется до следующего анализа.
Как сказали бы соседи-декларативщики — налицо поражение вирусом императивности и желание делать на ровном месте в 3-5-10 раз больше работы.
Но даже фиг с ним, как мы проверяем список (тем более что есть ещё варианты — например, any() с предикатом). Но чем тебе не понравилось решение со словарём???
Здравствуйте, netch80, Вы писали:
N>Здравствуйте, Константин, Вы писали:
К>>Плохо. В первую очередь доджно быть не pythonic, а читабельно. К>>P.S. IMHO все варианты в этой ветке менее понятны для беглого взгляда чем изначальный вариант. N>Очень странный подход. Вместо того, чтобы пользоваться вариантами, в которых чётко сказано, что делается — ты хочешь вариант, в котором написано, как оно делается (проходом по циклу), после чего ты:
Разве были варианты, где было бы сказано что делается? Если уж модифицировать код, то так:
def add_customer(c):
if has_customer_with_name(c.name):
_customers += c
N>Как сказали бы соседи-декларативщики — налицо поражение вирусом императивности и желание делать на ровном месте в 3-5-10 раз больше работы.
Мне эта фраза непонятна.
N>Но чем тебе не понравилось решение со словарём???
Нормальное решение. Возможно лучшее, а возможно и неприменимое (полную постановку задачи я не знаю)
Моё неприятие касается модификаций исходного варианта со списком
Здравствуйте, Константин, Вы писали:
К>>>P.S. IMHO все варианты в этой ветке менее понятны для беглого взгляда чем изначальный вариант. N>>Очень странный подход. Вместо того, чтобы пользоваться вариантами, в которых чётко сказано, что делается — ты хочешь вариант, в котором написано, как оно делается (проходом по циклу), после чего ты: К>Разве были варианты, где было бы сказано что делается?
Да. "Если есть элемент с таким свойством в списке" — это "что". Пройтись по списку с проверкой каждого — это "как".
К> Если уж модифицировать код, то так: К>
К>def add_customer(c):
К> if has_customer_with_name(c.name):
К> _customers += c
К>
Если has_customer_with_name() исполнена через явный цикл — это ещё одно "как", только скрытое. То же самое можно было бы сделать комментарием, не выделяя отдельно. Кстати, ты в проверке явно пропустил "not".
N>>Как сказали бы соседи-декларативщики — налицо поражение вирусом императивности и желание делать на ровном месте в 3-5-10 раз больше работы. К>Мне эта фраза непонятна.
Так я ж прямо перед ней расписал в подробностях, на чём именно делается больше работы.
N>>Но чем тебе не понравилось решение со словарём??? К>Нормальное решение. Возможно лучшее, а возможно и неприменимое (полную постановку задачи я не знаю) К>Моё неприятие касается модификаций исходного варианта со списком
Здравствуйте, netch80, Вы писали:
N>То же самое можно было бы сделать комментарием, не выделяя отдельно.
Я предпочитаю, чтобы код говорил сам не требовал комментария. Не понимаю, зачем он нужен для такой тривиальнейшей вещи, как приверить наличие имени.
N>Кстати, ты в проверке явно пропустил "not".
Я пропустил срочку с киданием исключения:
def add_customer(c):
if has_customer_with_name(c.name):
raise ValueError
_customers += c
N>>>Как сказали бы соседи-декларативщики — налицо поражение вирусом императивности и желание делать на ровном месте в 3-5-10 раз больше работы. К>>Мне эта фраза непонятна.
N>Так я ж прямо перед ней расписал в подробностях, на чём именно делается больше работы.
1.
def has_customer_with_name(name):
for e in _cusomers:
if e.name == name:
return True
return False
if has_customer_with_name(c.name):
raise ValueError
2. Вариант-сюрприз. Фильтрация списка оказывается на таковой. Оказыавается кидается исключение
def test_customer_exist(e):
if e.name == c.name:
raise ValueError
filter(test_customer_exist, _cusomers)
## читабельность ниже плинтуса
3. Вариант, отличный для haskell. Для Python — premature pessimization
if filter(lambda e: e.name == c.name, _cusomers):
raise ValueError
filter(lambda e: throw(ValueError) if с.name == e.name else None, _cusomers)
5. Нормальные варианты (в особенности второй). Пессимизация незначительная (слава богу не на алгоритмическом уровне). Читабельность неплохая.
for aux in (x for x in _customers if x.name == c.name):
raise ValueError
if c.name in (x.name for x in _customers):
raise ValueError
Разбор с for:
1) разбираешь код этого варианта, выделяешь цикл и действие в нём;
2) логически "сворачиваешь" этот цикл, выделяя, что же собственно планировалось сделать (а именно — найти клиента с тем же именем);
3) идёшь опять "вглубь" проверяя код на соответствие этому замыслу (нет ли плюх реализации).
После чего эта информация — что именно делалось — теряется до следующего анализа.
Разбор с generator:
1) разбираешь код этого варианта, выделяешь создание генератора и критерий создания
2) думаешь и понимаешь, что же собственно планировалось сделать (а именно — найти клиента с тем же именем);
3) опять смотришь проверяя код на соответствие этому замыслу (нет ли плюх реализации)
После чего эта информация — что именно делалось — теряется до следующего анализа.