Skip to content
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

Add YAML and JSON encoding functions #20

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

HELamer
Copy link

@HELamer HELamer commented Dec 7, 2023

No description provided.

Copy link

@robotomize robotomize left a comment

Choose a reason for hiding this comment

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

Отличная работа, но есть несколько моментов. В следующих работах обяхательно создавайте собственный репозиторий с проектом или делайте fork проекта яндекса
По работе, есть ошибка которую нужно исправить, возврат ошибки, все остальное ркоммендации

return nil
}

err = json.Unmarshal(jsonFile, &j.DockerCompose)
Copy link

@robotomize robotomize Dec 7, 2023

Choose a reason for hiding this comment

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

Отлично, что сразу десериализовали в нужную структуру


yamlFile, err := os.Open(j.FileOutput)
if err != nil {
fmt.Printf("ошибка при открытии файла %s: %s", j.FileOutput, err.Error())

Choose a reason for hiding this comment

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

Круто, что продумали некоторые вербоус действий, которые происходят в системе

}
}

defer yamlFile.Close()

Choose a reason for hiding this comment

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

Отлично, предусмотрели закрытие файлов после завершения функции

jsonFile, err := os.ReadFile(j.FileInput)
if err != nil {
fmt.Printf("ошибка при чтении %s файла: %s", j.FileInput, err.Error())
return nil

Choose a reason for hiding this comment

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

Вот тут есть момент, который нужно исправить во всех похожих случаях. Если мы обрабатываем ошибку, нужно ее вернуть return err

Все, что ниже просто дополнительная информация.
Не могу точно сказать прошли вы этот материал или еще нет, но если вы хотите добавить некоторый контекст в вашу ошибку, то лучше сделать так
return fmt.Errorf("ошибка открытия файла %s: %w", j.FileInput, err)
Тогда мы возвращаем ошибку, но добавляем в нее дополнительный контекст подставляя вместо глагола %w нашу ошибку. И уже этот текст ошибки залогировать наверху в main() где мы получим ошибку с более емким содержимым

Copy link
Author

Choose a reason for hiding this comment

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

Исправлено

@HELamer
Copy link
Author

HELamer commented Dec 7, 2023

Отличная работа, но есть несколько моментов. В следующих работах обяхательно создавайте собственный репозиторий с проектом или делайте fork проекта яндекса По работе, есть ошибка которую нужно исправить, возврат ошибки, все остальное ркоммендации

По этому вопросу. Я же вроде и сделал Fork репозитория яндекса, как написано в задании.

Copy link

@robotomize robotomize left a comment

Choose a reason for hiding this comment

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

Хорошо, что исправили, если будет время прочитайте подробнее про механизм обработки ошибок в Go и errorf
Но к сожалению нужно исправить маленькую историю с os.Create, прочитайте коммент


_, err = yamlFile.Write(yamlData)
if err != nil {
return fmt.Errorf("ошибка при записи данных в файл: %s: %w\n", j.FileOutput, err)

Choose a reason for hiding this comment

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

Да, все круто, но нам нужен тут \n
В данном случае fmt.Errorf() возвращает ошибку. Когда вы ее где нибудь наверху поймаете вы сможете отформатировать текст как захотите, поэтому \n лучше убрать
** Правка не обязательная

if err != nil {
fmt.Printf("ошибка при открытии файла %s: %s\n", y.FileOutput, err.Error())
fmt.Printf("создаем файл %s\n", y.FileOutput)
jsonFile, err = os.Create(y.FileOutput)

Choose a reason for hiding this comment

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

Смотрите, в этой задаче какая у нас логика идет

  • Сначала в программе ниже создаются входные файлы
  • Потом мы читаем файл и десериализуем в структуру
  • Потом мы эту структуру сериализуем в новый файл, который нужно создать
    В данном случае нам не нужно открывать выходной файл, которого еще нет, нам нужно сразу его создать и записать данные.
    То есть os.Open тут лишнее, сразу os.Create тогда и ошибку вы эту не поймаете. Сейчас она у вас видимо возникает, потому что файла нет, а вы пытаетесь его открыть.

Итого: нужно убрать os.Open и оставить только os.Create

Copy link
Author

Choose a reason for hiding this comment

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

Исправлено. Сначала подумал что все 4ре файла создаются сразу в начале main(), теперь увидел что там только два исходных

Copy link

@robotomize robotomize left a comment

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.

3 participants