Skip to content

Borodin Ilya #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Borodin Ilya #43

wants to merge 25 commits into from

Conversation

ilyaborodin
Copy link

@ilyaborodin ilyaborodin commented Nov 10, 2019

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

Последние сутки? Пора браться за задания)

Сделано:

  • 1-ая итерация
  • 2-ая итерация
  • 3-ая итерация
  • 4-ая итерация
  • 5-ая итерация
  • 6-ая итерация(выполнена частично, описание в readme)
  • Задания по практическим занятиям
  • Задания по гиту

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


def parsing_args():
"""Парсим аргументы"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

комментарии на русском лучше, чем транслитом, но на английском языке они лучше всего :)

Copy link
Author

Choose a reason for hiding this comment

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

Принял)

App/Portal.py Outdated
except Exception as e:
print("Problems with rss processing")
logging.error(str(e))
exit()
Copy link
Collaborator

Choose a reason for hiding this comment

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

у программы такого размера может быть одна точка выхода, в данном случае их минимум 7
тут имеет смысл сделать одну точку выхода из програмы (примерно рядом с точкой входа)

Copy link
Author

Choose a reason for hiding this comment

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

Поправил, спасибо

self.links = []
self.clear_text()

def pars_date(self, struct):
Copy link
Collaborator

Choose a reason for hiding this comment

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

А чем datetime или аналогичные библиотеки не подходит для этой задачи?
или я что-то не до конца понимаю

logging.warning("Problems with tag parsing:\n" + str(e))

def __str__(self):
string = "Channel name: {0}\n" \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Это мое субъективное мнение, но мне кажется, что это слишком "тяжелая" логика для метода "str"
Лично я бы вынес это в отдельный метод и не использовал для этих задач str
но это мое мнение :)



class RSSListener:
""""""
Copy link
Collaborator

Choose a reason for hiding this comment

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

док стринг

delete_list.append(new_d)
for new_d in delete_list:
date_handler[date].remove(new_d)
with open("./Cache/" + date, 'wb') as f:
Copy link
Collaborator

Choose a reason for hiding this comment

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

в различных операционных системах могут отличаться разделители в пути. Советую не хардкодить такие вещи, а пользоваться функционалом os.path

@ilyaborodin ilyaborodin changed the title [WIP] Borodin Ilya Borodin Ilya Dec 4, 2019
@AlexeiBuzuma
Copy link
Collaborator

# rss-reader https://news.tut.by/rss/world.rss --help
unsupported format character 'y' (0x79) at index 49
The program suddenly completed its work

@AlexeiBuzuma AlexeiBuzuma added [3-4 iteration] Minimal requirements checked and removed Hard Deadline [Deadline] Iteration 1-2 This is a marker for first and second iterations deadline. labels Dec 11, 2019
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