Skip to content

Conversation

@BoryaD
Copy link

@BoryaD BoryaD commented Nov 8, 2019

No description provided.

@AlexeiBuzuma AlexeiBuzuma added the [Deadline] Iteration 1-2 This is a marker for first and second iterations deadline. label Nov 17, 2019
@@ -0,0 +1,11 @@
import setuptools

setuptools.setup(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

This package should export CLI utility named rss-reader

if self.cursor:
logger = logging.getLogger('rss_reader')
logger.error("This is singleton class. Use get_cursor")
exit()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

exit()
Cache.conn = sqlite3.connect(file_path)
Cache.cursor = Cache.conn.cursor()
Cache.cursor.execute('''CREATE TABLE IF NOT EXISTS news(id INTEGER PRIMARY KEY,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

    @staticmethod
    def get_cursor():
        """Static access method. """
        if Cache.cursor is None:
            Cache()
        return Cache.cursor

@BoryaD BoryaD changed the title [WIP] Dashko Boris Dashko Boris Nov 30, 2019
@BoryaD BoryaD changed the title Dashko Boris [WIP]Dashko Boris Dec 1, 2019
@BoryaD BoryaD changed the title [WIP]Dashko Boris Dashko Boris Dec 1, 2019
@AlexeiBuzuma
Copy link
Collaborator

при попытке конвертации в html

# rss-reader https://news.tut.by/rss/world.rss --to-html /data/news.html
Traceback (most recent call last):
  File "/usr/local/bin/rss-reader", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.8/site-packages/rss_reader/rss_reader.py", line 61, in main
    news.create_html(args.to_html)
  File "/usr/local/lib/python3.8/site-packages/rss_reader/news.py", line 173, in create_html
    with open(filename, 'w', encoding="utf-8") as html_file:
UnboundLocalError: local variable 'filename' referenced before assignment

@BoryaD
Copy link
Author

BoryaD commented Dec 11, 2019

при попытке конвертации в html

# rss-reader https://news.tut.by/rss/world.rss --to-html /data/news.html
Traceback (most recent call last):
  File "/usr/local/bin/rss-reader", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.8/site-packages/rss_reader/rss_reader.py", line 61, in main
    news.create_html(args.to_html)
  File "/usr/local/lib/python3.8/site-packages/rss_reader/news.py", line 173, in create_html
    with open(filename, 'w', encoding="utf-8") as html_file:
UnboundLocalError: local variable 'filename' referenced before assignment

Да, косяк был, исправил, перезалить есть смысл, или уже все?

@AlexeiBuzuma
Copy link
Collaborator

В выводе новостей (в консоль, в пдф и fb2) отсутствуют картинки и ссылки на них

@AlexeiBuzuma AlexeiBuzuma removed Hard Deadline [Deadline] Iteration 1-2 This is a marker for first and second iterations deadline. labels Dec 12, 2019
@AlexeiBuzuma
Copy link
Collaborator

  1. Отсутствуют юнит тесты
  2. Очень запутывающий подход используется в функции get_cursor. В данном случае соединение и курсор хранятся напрямую в аттрибутах класса, что может вызвать много багов и запутывать людей, которые будут читать код (для всех объектов класса Cursor будет одинаковые значения этих атрибутов). В данном случае функциональность фактически похожа на использование глобальных переменных, только еще больше запутывает из-за использования аттрибутов класса.
  3. в функции init класса News есть очень много логики. Фактически, эта функция предназначена просто для инициализации объекта, но в данном случае эта функция хранит много логики. Советую всю эту логику вынести из init
  4. В решении есть две функции, которые фактически занимаются выводом новости в консоль. в классе Cache и в классе News. для этой функциональности есть смысл использовать один и тот же код (в целом класс Cache не должен отвечать за вывод новостей в консоль, а должен заниматсья кэшированием :) )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants