[+] Added 21 lesson task #30
Conversation
| // IFilms это интерфейс для работы с фильмами | ||
| type IFilms interface { |
There was a problem hiding this comment.
не, неудачно.
films.IFFilms - не звучит.
films.Interface - универсальное решение, если ничего не приходит на ум. Встречается даже в стандартной библиотеке.
There was a problem hiding this comment.
И, кстати, чаще употребляется movie.
| // connect возвращает контекст и соединение к БД | ||
| func (pg *Pg) connect() (context.Context, *pgxpool.Pool, error) { |
There was a problem hiding this comment.
не очень понимаю смысл параметра-контекста.
эта функция вызывается единожды и прерывать её вряд ли потребуется.
лишнее, на мой взгляд.
| return ctx, db, nil | ||
| } |
There was a problem hiding this comment.
и возвращаемый контекст не понятно куда можно применить. вряд ли эта функция может быть частью конвейера или типа того.
| ctx, db, err := pg.connect() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| defer db.Close() |
There was a problem hiding this comment.
всё понятно.
так делать не надо. подключаться к БД следует однажды, и переиспользовать объект БД впоследствии.
PGX беден на документацию, и я не смог найти прямого указания, но это так.
рекомендации для databse/sql - PGX работает и через него тоже:
Although it’s idiomatic to Close() the database when you’re finished with it, the sql.DB object is designed to be long-lived. Don’t Open() and Close() databases frequently. Instead, create one sql.DB object for each distinct datastore you need to access, and keep it until the program is done accessing that datastore. Pass it around as needed, or make it available somehow globally, but keep it open. And don’t Open() and Close() from a short-lived function. Instead, pass the sql.DB into that short-lived function as an argument.
There was a problem hiding this comment.
да, я об этом думал, когда делал отдельный метод для получения коннекта, но подумал, что это может быть признано ошибкой, т.к. в лекции говорилось, что соединения с БД не бесконечны и их надо закрывать.
Тут зависит от того, как пакет предполагается использовать. В принципе, да, можно было бы сохранять соединение и переоткрывать по истечении некоторого времени, чтобы не держать его слишком долго.
There was a problem hiding this comment.
Всё не так ) Вспоминайте лекцию - я несколько раз повторил, что это не соединение! Это - пул соединений, но лучше думать об этом объекте как о БД.
Соединения открываются и закрываются во время выполнения запросов. Методом Acquire(). Так что тут вы закрываете саму БД, или типа того.
There was a problem hiding this comment.
Вручную надо закрывать только batch. При использовании database/sql ещё и prepared statement.
Пул закрывать не надо.
Поэтому я и говорил, что предпочитаю думать об этом объекте как о БД как таковой.
There was a problem hiding this comment.
И да, Acquire() и Close() при выполнении запросов вызываются под капотом. Так, для ясности.
| ctx, db, err := pg.connect() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| defer db.Close() |
There was a problem hiding this comment.
короче, вот этот блок лишний везде
| if studioID > 0 { | ||
| rows, err = db.Query(ctx, ` | ||
| SELECT id, title, year, profit, pgrating, studio_id | ||
| FROM films | ||
| WHERE studio_id = $1`, | ||
| studioID, | ||
| ) | ||
| } else { | ||
| rows, err = db.Query(ctx, ` | ||
| SELECT id, title, year, profit, pgrating, studio_id | ||
| FROM films`, | ||
| ) | ||
| } |
There was a problem hiding this comment.
ага, я ждал один запрос
типа WHERE $1 = 0 OR studio_id = $1
но это не очевидно, конечно, хотя и ощутимо лучше
| if err := testdb.loadSQL(string(file)); err != nil { | ||
| fmt.Println(err.Error()) | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
функцию loadSQL логичней было бы отнести к тестовому файлу
|
|
||
| func TestDb_films(t *testing.T) { | ||
| // Добавляем фильм | ||
| addFilms := []films.Film{ |
There was a problem hiding this comment.
movies (или films), но никак не глагол
|
|
||
| // Изменяем название фильма и проверяем есть ли изменения | ||
| updFilm := addFilms[0] | ||
| updFilm.Title = "Pulp Fiction 2" |
За основу взята полная схема из задания №20, но интерфейс реализует только операции с фильмами (если я правильно понял, остальные таблицы не задействованы).
Для загрузки тестового SQL-дампа написал функцию loadSQL, которая вообще говоря не нужна в пакете, но немного облегчает тестирование.
Тест TestDb_films() последовательно тестирует функции AddFilms, UpdateFilm и DeleteFilm - мне так показалось логичнее и проще. Функция Films() тестируется отдельно.