ShadPython - 2 - Yaraslau Zadvorny - CS - Minsk

https://contest.yandex.ru/contest/1147/run-report/716649/

[slava.zadvorny, 2015-06-26 16:19:02]

Добрый день. 
Высылаю свое решение второго домашнего задания.
С уважением, Ярослав Задворный (1 курс, филиал г. Минска)

[shad.python, 2015-06-26 17:33:58]

https://codereview.appspot.com/251770043/diff/1/main.py
File main.py (right):

https://codereview.appspot.com/251770043/diff/1/main.py#newcode46
main.py:46: words = tokenize(line)
Я бы рекомендовал сделать, чтобы эта функция сразу получала на вход
правильно отфильтрованный список/генератор токенов, а не сама их
делала/фильтровала (возможно, в зависимости от сложных опций).
Тогда, если нужно будет использовать эту функцию в новой задаче, ее не
придется переделывать.

https://codereview.appspot.com/251770043/diff/1/main.py#newcode80
main.py:80: def frequency_main(depth, text, chains):
В функции find_chains вы уже посчитали почти всю необходимую информацию, а потом
вы ее выкидываете и здесь считаете все заново. Из-за этого код дублируется и его
тяжело воспринимать. Нужно переделать, чтобы избавиться от этой проблемы.

https://codereview.appspot.com/251770043/diff/1/main.py#newcode165
main.py:165: def word_upper(word):
Можно использовать метод unicode.title()

https://codereview.appspot.com/251770043/diff/1/main.py#newcode173
main.py:173: def word_lower(word):
Мне кажется, понижать только первую букву в слове нет смысла. Не-титульные слова
должны быть либо полностью в нижнем регистре, либо полностью в верхнем
(например, аббревиатуры). Поэтому можно просто использовать unicode.lower()

https://codereview.appspot.com/251770043/diff/1/main.py#newcode182
main.py:182: chains = find_chains(depth, text)
Не очень хорошо, что функция обучения модели (сбора вероятностей) находится
прямо в функции генерации. Для того, чтобы сгенерировать несколько текстов на
основе одной модели, придется каждый раз заново обучать ее. Лучше разделить эти
действия.

https://codereview.appspot.com/251770043/diff/1/main.py#newcode206
main.py:206: a = random.random()
Действие "выбрать случайный элемент из распределения" может пригодиться
и в других задачах - я бы рекомендовал вынести его в отдельную функцию.
К тому же это поможет упростить этот код.

https://codereview.appspot.com/251770043/diff/1/main.py#newcode258
main.py:258: for line in text:
Ввод/вывод и содержательные действия лучше отделять друг от друга. Это позволит
в дальнейшем этот код переиспользовать, даже если формат ввода и вывода
поменяется.

https://codereview.appspot.com/251770043/diff/1/main.py#newcode504
main.py:504: main()
Для того, чтобы вашу программу можно было импортировать как модуль, нужно весь
глобальный код писать внутри такой конструкции: 

if __name__ == '__main__':
    ...

[slava.zadvorny, 2015-07-04 08:41:05]

Добрый день.

Высылаю новую версию решения второго домашнего задания, а также сгенерированный
текст. Генератор обучался на отывке из "Алисы в стране чудес" с глубиной цепочек
3.

С уважением, Ярослав Задворный (1 курс, филиал г. Минска)

[shad.python, 2015-07-06 19:01:42]

Ещё замечания по-мелочи.

--
Григорий Носырев

https://codereview.appspot.com/251770043/diff/20001/main.py
File main.py (right):

https://codereview.appspot.com/251770043/diff/20001/main.py#newcode8
main.py:8: locale.getdefaultlocale()[1]
Зачем? Лишний вызов, нигде потом не используется

https://codereview.appspot.com/251770043/diff/20001/main.py#newcode106
main.py:106: line = line[0:(len(line) - 1)]
или line[:-1]

https://codereview.appspot.com/251770043/diff/20001/main.py#newcode115
main.py:115: b = bool(place == 0 and line[len(key)] == ' ')
b и c ничего не говорящие названия, назовите их как-нибудь по-человечески

https://codereview.appspot.com/251770043/diff/20001/main.py#newcode140
main.py:140: array_probability = probability.items()
чем два раза вызывать sorted, можно один раз сразу отсортировать массив
array_probability.sort()
Плюс тут сортировка вообще не нужна.

https://codereview.appspot.com/251770043/diff/20001/main.py#newcode150
main.py:150: for key, diction in sorted(probability.items()):
дикция?)

https://codereview.appspot.com/251770043/diff/20001/main.py#newcode159
main.py:159: print ('  ' + key + ': ' + "%1.2f" % (item)).encode('utf-8')
в таких случаях удобнее пользоваться функцией .format у строки

https://codereview.appspot.com/251770043/diff/20001/main.py#newcode202
main.py:202: if text[len(text)-1] == '.':
или text[-1]

https://codereview.appspot.com/251770043/diff/20001/main.py#newcode362
main.py:362: class Chains_Tester(object):
Тесты лучше реализовывать через модуль unittest, это позволяет сильно
автоматизировать процесс тестирования

https://codereview.appspot.com/251770043/diff/20001/main.py#newcode436
main.py:436: if (args.task == "tokenize"):
такую логику лучше реализовать через субпарсеры и отдельные функции. Подробнее
прочитать про них можно в документации к argparse

https://codereview.appspot.com/251770043/diff/20001/main.py#newcode464
main.py:464: count_dots = 0
явно напрашивается вынос подсчёта статистики в отдельную функцию

https://codereview.appspot.com/251770043/diff/20001/main.py#newcode486
main.py:486: tokenize_tester = Tokenize_Tester(1)
классы принято называть ВотТак, без подчёркиваний

[slava.zadvorny, 2015-07-09 21:40:52]

Добрый день.

И еще одна попытка сдать второе домашнее задание)

С уважением, Ярослав Задворный (1 курс, филиал г. Минска)