Покритикуйте пет-проект

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

На удаленку такой как я за 50 им не нужен? :)

Проблемы ПМ-зависимых.

Код бекенда - полное говно, игнорировать везде error не надо, да и в целом эталонная лапша

Согласен, много мест корявых, но как мне кажется, для прототипа вполне нормально, могу ошибаться.

игнорировать везде error
везде

Не согласен, везде где принципиально важно - проверки есть.

По клиенту скажешь что-нибудь?

Не согласен, везде где принципиально важно - проверки есть.

Везде принципиально важно

Не согласен, везде где принципиально важно - проверки есть.

Вот к примеру у тебя эмуляция дефолтного конфига.

func GetConfig() Config {
	var config = Config{Host: "", Port: "8080", Secret: "lol"}
	file, err := os.Open("./config.json")
	defer file.Close()
	if err == nil {
		log.Println(json.NewDecoder(file).Decode(&config))
	}
	return config
}

Что тут не так:

• Нет корректной проверки, все ли хорошо с файлом. Может быть, он не открывается, потому что на него нет прав?
• Конфиг может быть открыт, но декодирован неуспешно.

Немного более корректная реализация:

func GetConfig() (*Config, error) {

	var config = Config{
		Host:   "",
		Port:   "8080",
		Secret: "lol",
	}

	file, err := os.Open("config.json")
	defer file.Close()

	if err != nil && !os.IsNotExist(err) {
		return nil, err
	}

	err = json.NewDecoder(file).Decode(&config)
	if err != nil {
		return nil, err
	}

	return &config, nil
}

Константы такие, будто бы у тебя буквы на клавиатуре платные. Нафига?

Для прода твой вариант лучше, для демо мой мне больше нравится.

При ошибке декодирования, насколько я понимаю функция вернёт значения по умолчанию, смысл тогда от лишних проверок и возврата ошибки если приложение всё равно запустится?

у тебя буквы на клавиатуре платные

Мне казалось так в го-сообществе чаще всего и пишут, разве нет?

смысл тогда от лишних проверок и возврата ошибки если приложение всё равно запустится?

Пользователь узнает, что конфиг некорректный сразу. А не после долгих разборок, почему он не применяется.

Мне казалось так в го-сообществе чаще всего и пишут, разве нет?

Нет

Для прода твой вариант лучше, для демо мой мне больше нравится.

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

Это же касается других мест. Игнорирование ошибок = неопределенное поведение = куча странных и неприятных ситуаций.

За хайп. Собственно лет через 5 Го сегодняшнего точно не будет.

Думаешь таки генерики завезут?

Кстати у нас ушли от Эрланга в го из за того, что эрланг по словам разработчиков «говно»...

И правильно сделали. Чтобы делать что-то на эрланг нужно быть с повернутой психикой.

В этой функции ужасно вообще все

1. Получишь панику в defer если с файлом что-то не так

2. Ошибка из file.Close() никак не обрабатывается

3. Путь к конфигу захардкожен

4. Условие проверки ошибки открытия файла некорректное

5. Корректно использовать config := &Config{}, без var

6. json.NewDecoder нужен для потоковой обработки жсона, здесь правильно скопировать данные с файла в буфер и использовать json.Unmarshal

7. Функция задизайнена так, что при каждом вызове, конфиг будет заново перечитываться

1. Получишь панику в defer если с файлом что-то не так

Я кстати специально глянул код os, не получишь

2. Ошибка из file.Close() никак не обрабатывается

Да, не критично. 1) От этого сложно огрести 2) Все равно обычно максимум, что можно сделать - залоггировать эту ошибку

3. Путь к конфигу захардкожен

Да, не критично

4. Условие проверки ошибки открытия файла некорректное

В чем? Я не проверял, но на глаз всё ок.

5. Корректно использовать config := &Config{}, без var

Абсолютно никакой разницы кроме внешнего вида

6. json.NewDecoder нужен для потоковой обработки жсона, здесь правильно скопировать данные с файла в буфер и использовать json.Unmarshal

Неверно. Зачем создавать промежуточный буфер, когда json сам это может прочитать? Предлагаемое тобой решение медленней и потребляет лишнюю память.

7. Функция задизайнена так, что при каждом вызове, конфиг будет заново перечитываться

Естественно, ведь вызывать её нужно только тогда, когда нужно перечитать конфиг - при запуске и reload. Так ли это в сабжевом коде я не смотрел, но по-хорошему делать нужно примерно так.

конфиг некорректный

Так в лог же падает ошибка

Хреновый подход
Нет

Принял, спасибо.

при каждом вызове, конфиг будет заново перечитываться

Конфиг читается один раз, при запуске.

Это где такое? Для сравнения - работая плюсовиком (не джуниором), я считал 70к за счастье для Ростова.

Только лично

У Эрланга куча минусов

А что за плашки под картинками? Категории? Почему две?

Я кстати специально глянул код os, не получишь

Получишь, потому, что тебе придет nil, на котором вызовешь Close()

Да, не критично

Критично. Ошибку ты должен пробросить дальше

В чем?

В том, что если ошибка произошла, но файл существует, ты эту ошибку опускаешь

Неверно. Зачем создавать промежуточный буфер, когда json сам это может прочитать?

В том, что они работают не эквивалентно. Потоковый декодер спокойно проглатывает незакрытые кавычки и в нем есть другие нюансы. Плавали, знаем.

Естественно, ведь вызывать её нужно только тогда, когда нужно перечитать конфиг

Перечитывать конфиг - это с вероятностью 99% ошибка в дизайне приложения. Конкретно этого - точно.

Получишь, потому, что тебе придет nil, на котором вызовешь Close()

Не получу. Еще раз, разжевываю на пальцах:

1) go позволяет делать вызывы с pointer receiver

2) в коде os.Close есть проверка на nil https://golang.org/src/os/file_unix.go?s=6716:6744#L212

Критично. Ошибку ты должен пробросить дальше

К каким проблемам в каких ситуациях может это привести?

В том, что если ошибка произошла, но файл существует, ты эту ошибку опускаешь

Мне нечего добавить к тому, что ты просто не умеешь читать

package main

import (
	"fmt"
	"os"
)

func main() {

	file, err := os.Open("config.json")
	defer file.Close()

	if err != nil && !os.IsNotExist(err) {
		fmt.Println("got error", err)
		return
	}

	fmt.Println("no error")
}

der@nyanpad /tmp % go run main.go 
no error
der@nyanpad /tmp % touch config.json
der@nyanpad /tmp % go run main.go   
no error
der@nyanpad /tmp % chmod a-rwx config.json 
der@nyanpad /tmp % go run main.go         
got error open config.json: permission denied

В том, что они работают не эквивалентно. Потоковый декодер спокойно проглатывает незакрытые кавычки и в нем есть другие нюансы. Плавали, знаем.

Примеры в студию, звучит как сказки.

Перечитывать конфиг - это с вероятностью 99% ошибка в дизайне приложения

В фортунки, что еще сказать

Покритикуйте пет-проект

Пожалуйста, критика лор-стайл: «Ваш пет-проект говно, вы ничего не понимаете в пет-проектах. Откуда вы такие лезите! Код не смотрел».

Не получу. Еще раз, разжевываю на пальцах:

Очевидно же, что захардкожена проверка для такой ситуации, задумано, что defer должен стоять после обработки ошибки

К каким проблемам в каких ситуациях может это привести?

У тебя ошибка в приложении и ты ее не хендлишь, это может привести к любым проблемам

Мне нечего добавить к тому, что ты просто не умеешь читать

Ну так это типичная ошибка из-за двойного отрицания. Есть функция os.IsExist(), небось в блокноте без интелисенса пишешь, потому и не заметил

Примеры в студию, звучит как сказки.

https://play.golang.org/p/2oLfk8Wjhav

В фортунки, что еще сказать

Я у тса не вижу локиги грейсфул рестарта, переинициализации подключения к бд и кучи других вещей, которые нужно было бы сделать для перезагрузки конфига

Я у тса не вижу локиги грейсфул рестарта, переинициализации подключения к бд и кучи других вещей, которые нужно было бы сделать для перезагрузки конфига

Так зачем это делать? Ты же назвал reload «ошибкой в дизайне приложения».

https://play.golang.org/p/2oLfk8Wjhav

Интересно. Decoder не использовал. Вероятно, нужно проверять еще и More() == false https://play.golang.org/p/xWRiRQqwDtz

Очевидно же, что захардкожена проверка для такой ситуации, задумано, что defer должен стоять после обработки ошибки

Да. У меня были сомнения и сейчас загуглил - так и есть. Хотя с левыми кривыми либами все равно нужно быть осторожней. Там же описан список возможных ситуаций, когда file.Close вернет ошибку. Из него следует, что defer file.Close() делать вполне допустимо.

Ну так это типичная ошибка из-за двойного отрицания. Есть функция os.IsExist(), небось в блокноте без интелисенса пишешь, потому и не заметил

Действительно, если ты не умеешь читать, то и документацию ты тоже не умеешь читать. IsExist это для совсем других ошибок.

package main

import (
	"fmt"
	"os"
)

func main() {

	file, err := os.Open("config.json")
	defer file.Close()

	if err != nil && os.IsExist(err) {
		fmt.Println("got error", err)
		return
	}

	fmt.Println("no error")
}

der@nyanpad /tmp % go run main.go
no error
der@nyanpad /tmp % touch config.json
der@nyanpad /tmp % go run main.go   
no error
der@nyanpad /tmp % chmod a-rwx config.json
der@nyanpad /tmp % go run main.go         
no error

Но действительно, зачем читать документацию, автодополнение в говноIDE плохого не посоветует. Тем временем даже блокнот показывает документацию метода при автодополнении.

Так зачем это делать?

Так а зачем тогда перезагружаемый конфиг?

Интересно. Decoder не использовал

Не интересно, а логично, вед декодер нужен для потокового чтения. Уже сто раз обсосано: https://ahmet.im/blog/golang-json-decoder-pitfalls/

Типичный пример как преждевременной оптимизацией попался на стопяцот граблей, сэкономил килобайт памяти вычитав конфиг в буфер, зато

Хотя с левыми кривыми либами все равно нужно быть осторожней

Семантика Close задумана авторами языка чтобы вызывался после проверки на ошибку, то, что один раз это не вызывает панику - лишь везение, можно миллион раз перебегать дорогу на красный

IsExist это для совсем других ошибок.

Да оно там вообще не нужно. Если открытие файла вызвало ошибку, дальше вообще нефиг делать, абы лепить в код что попало

Семантика Close задумана авторами языка чтобы вызывался после проверки на ошибку, то, что один раз это не вызывает панику - лишь везение, можно миллион раз перебегать дорогу на красный

Да, все так, но os.file.Close никогда не будет вызывать панику.

Да оно там вообще не нужно. Если открытие файла вызвало ошибку, дальше вообще нефиг делать, абы лепить в код что попало

Если ситуация «нет конфига, использовать дефолтный» корректна, то ситуация вполне нормальна. Если ты, конечно, умеешь читать доки по IsNotExist

Да, все так, но os.file.Close никогда не будет вызывать панику.

Ну так ошибку из Close ты все-равно не обрабатываешь, потому это проходит незаметно для тебя

Если ситуация «нет конфига, использовать дефолтный» корректна, то ситуация вполне нормальна

Не рекомендую использовать дефолтные значения, при деплое очень напрягает искать ошибки

Не рекомендую использовать дефолтные значения, при деплое очень напрягает искать ошибки

Это уже совсем другое дело.

Покритикуйте

Критикую! Тестов нету! дальше не смотрел

os.file.Close никогда не будет вызывать панику.

ложь, брехня и провокация. А не обрабатывать ошибку при закрытии файла доступного на запись - свинство.

ложь, брехня и провокация

нет.

А не обрабатывать ошибку при закрытии файла доступного на запись - свинство.

В чтении конфигов, внезапно, нет записи в файл.

В чтении конфигов

внезапно, тогда будет не плохо открыть его только на чтение

нет записи в файл.

про нет или есть, ты в своем сообщении не писал.

нет

https://www.joeshaw.org/dont-defer-close-on-writable-files/

Я приводил эту ссылку, лол. Там есть список возможных ошибок Close. Ошибок. Паник в списке нет, ты их никак не добьешься, даже при желании.

on writable files

Там же есть

про нет или есть, ты в своем сообщении не писал.

GetConfig

Я приводил эту ссылку, лол

тогда чего за вопросы, подкладывай солому, что ли.

Там же есть

куда я реплаил - нет. А тс потом может решить, что если ты пишешь «никогда», значит «никогда», а не, ну иногда может, но не тут.

Там есть список возможных ошибок Close.

это не повод открыть конфиги на запись, особенно если писать в них не собираешься, разве это не очевидно ?

А тс потом может решить, что если ты пишешь «никогда», значит «никогда», а не, ну иногда может, но не тут.

Да я уже 10 раз в треде сказал, что оставил эту фичу ТС в коде только потому, что был не уверен, но nikolnik в этом вопросе оказался более правым. Тем не менее, это не так критично и к паникам не приведет.

это не повод открыть конфиги на запись

таки да

Критичность в том, что когда чувак делает проект чтобы потом пойти работать в команде, в этой команде могут по рукам надавать.

В его коде это одна из наименьших проблем, практически без побочных негативных эффектов. Напомню, он там кучу os.Remove без проверок на ошибки фигачит.

Так в этом и суть, если намекнуть, что даже мельчайшую часть надо обмазывать проверками на ошибки, то моменты больше будут уже очевидны.