Skip to content

Review #1

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 27 commits into
base: empty
Choose a base branch
from
Open

Review #1

wants to merge 27 commits into from

Conversation

vsubhuman
Copy link
Collaborator

No description provided.


def get_lines(self):
if not self.__fmt:
self.__recognize_format(self.__path)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

def get_lines(self):
  fmt = self.__get_format()
  ...

def __get_format(self):
  if not self.__fmt:
    self.__fmt = codec.get_string_to_recognize(self.__path)
  return self.__fmt

if not self.__fmt:
self.__recognize_format(self.__path)
if self.__last_change < self.__check_update():
with open(r'{0}'.format(self.__path), 'r', encoding='{}'.format(self.__fmt), errors='replace') as file:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Блин, ну я ж тебе писал уже

r'{0}'.format(x) == '{0}'.format(x) == x
'{}'.format(x) == x

"""Эта функция должна была обновлять текст на вкладке"""
self.txt.config(state='normal')
self.txt.insert(tkinter.END, self.document.get_lines())
self.txt.config(state='disabled')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Стоит проверять, что документ чота вернул, прежде чем пытаться обновлять табу:

text = self.document.get_lines()
if text:
  self.txt.config(state='normal')
  self.txt.insert(tkinter.END, text)
  self.txt.config(state='disabled')

def get_lines(self):
if not self.__fmt:
self.__recognize_format(self.__path)
if self.__last_change < self.__check_update():
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

last_processed_change = self.__last_change
current_last_change = os.stat(self.__path).st_mtime
if current_last_change > last_processed_change:
  try:
    ...
  except:
    <handle>
  else:
    self.__last_change = current_last_change

return self.__log_content
else:
self.__log_content = ''
return self.__log_content
Copy link
Collaborator Author

@vsubhuman vsubhuman Dec 24, 2017

Choose a reason for hiding this comment

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

Какую цель выполняет поле __log_content?

if current_last_change > last_processed_change:
  with open(...) as f:
    text = f.read()
    self.__tail = f.tell()
    return text
return None

return self.__list_of_tab


list_of_tab = ListOfTab()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Это глобальная переменная. Тот факт, что ты её реализовал без слова global не делает её лучше. Думай, как сделать программу без неё.

else:
self.__log_content = ''
return self.__log_content

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Обработка ошибок?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants