Pull to refresh

Comments 6

Чуток конструктивной критики:


Обработка ключей командной сроки

Я не знаю как в Tcl, но в Python для этого есть модуль getopt: https://docs.python.org/2/library/getopt.html


def parce_string(line):

Разбор записей фиксированного формата проще и быстрее всего делается через unpack: https://docs.python.org/2/library/struct.html


def insert(**kwargs):

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


Лучше использовать параметры SQL запроса:


conn.execute("INSERT INTO `foo` (foo, bar) VALUES (?, ?)", (foo_value, bar_value))

А чтобы сформировать сам запрос, циклы тоже использовать не обязательно. Можно оперировать списками:


q_keys = kwargs.keys()
q_values = kwargs.values()

q_key_list = ", ".join([str(k) for k in q_keys]
q_placeholder_list = ", ".join(['?' for k in q_keys])

query = "INSERT INTO `cdr` (%s) VALUES (%s)" % (q_key_list, q_placeholder_list)
connection.execute(query, q_values)

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


Кроме собственно кода, я бы ещё добавил как минимум индексы к таблице, ротацию логов и хотя бы рудиментарную обработку ошибок. Поскольку у вас CentOS 7, можно просто добавить systemd unit и получить две последние плюшки практически безвозмездно. :)

Я не знаю как в Tcl, но в Python для этого есть модуль getopt: docs.python.org/2/library/getopt.html

Да, я про getopt в курсе, и для тикля есть подобные, просто в данном случае для 2-3 опций использование дополнительного пакета счёл не целесообразным :)
Склеивать строку запроса из ключей вместе со значениями не очень хорошая идея. В вашем случае SQL инъекции можно не бояться, но если вдруг на входе прилетит лишняя запятая или кавычка, то запрос сломается и вы об этом даже не узнаете.

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

В моём случае подойдёт вариант со списками, спасибо за подсказку. Как я понял данный код:
q_key_list = ", ".join([str(k) for k in q_keys])

Мы перебираем список «for k in q_keys» и из его элементов преобразованных в строку «str(k)» лепим опять же строку с разделителем ", ".

Поскольку у вас CentOS 7, можно просто добавить systemd unit и получить две последние плюшки практически безвозмездно

Так и сделано, сервис запускается через systemd.

Спасибо за комментарий!

Есть еще argparse с дополнительными плюшками вроде документации, дефолтных значений, кастов к типам и прочим

Да, я про getopt в курсе, и для тикля есть подобные, просто в данном случае для 2-3 опций использование дополнительного пакета счёл не целесообразным :)

Лучше сразу использовать готовые модули, это хорошая привычка. Потом всё равно придётся переписывать, когда надо будет добавить ещё один параметр. :)


Тут набор данных строго регламентирован, лишнего не прилетит, если только не ошибки с порта.

а) ошибки из порта, б) ошибки в коде, ц) раз в год и незаряженное ружьё стреляет. Ошибки в коде легко могут возникнуть, если надо будет поменять формат CDR или вообще кто-нибудь решит приспособить ваш скрипт для другой модели АТС. Да вы же сами и решите, потом когда-нибудь.


Если же взять за правило всегда использовать параметры SQL, то можно не думать о многих проблемах потом. Ещё одна хорошая привычка. :)


Мы перебираем список «for k in q_keys» и из его элементов преобразованных в строку «str(k)» лепим опять же строку с разделителем ", ".

Всё правильно, но обратите внимание — всё это используется для того, чтобы построить список полей для вставки и список позиционных параметров. Сами значения передаются в execute(), что и обеспечивает отсутствие проблем с некорректными значениями.


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

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

Не вижу ничего странного, lua вот даже изменяемые таблицы в качестве ключей позволяет, тогда как Python пытается это предотвратить (не то чтобы я не смог запихнуть экземпляр изменяемого словаря в качестве ключа, если бы захотел: нужно только правильно определить хэш‐функцию (id пойдёт) и отношение равенства (словари равны, если это один и тот же словарь — так делает lua)). В C++ и rust вы тоже можете использовать практически любой тип в качестве типа ключа.

Sign up to leave a comment.

Articles