-
Notifications
You must be signed in to change notification settings - Fork 6
Conversation
17fb7b9
to
c5aeaef
Compare
backend/integration/src/main/scala/ru/tinkoff/tcb/mockingbird/edsl/MarkdownInterpretator.scala
Outdated
Show resolved
Hide resolved
backend/integration/src/main/scala/ru/tinkoff/tcb/mockingbird/example/HttpStubSet.scala
Outdated
Show resolved
Hide resolved
* @param count | ||
* количество заглушек, отображаемых на странице | ||
*/ | ||
final case class StubFetchParams( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не очень понятен смысл этих моделей, почему просто не передавать несколько параметров в метод?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Мой главный аргумент такой: API остаётся стабильным при необходимости добавить новые параметры
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
При этом в случае добавления параметра всё равно придётся править все места, где происходит вызов (ну, в данном случае где собирается модель)... Ещё и аллокация паразитная
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше править меньше мест. По факту:
- в обоих вариантах правки: реализации, вызовы
- в варианте с доп.параметром добавляется: определение метода в интерфейсе + в каждой реализации
добавлю ещё, что после n-ного параметра возникнет соблазн задать дефолтные значения, чтобы упростить вызов. и тут могут быть неоднозначности с оверрайдами
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Оверрайды зачастую не нужны когда можно обойтись дефолтными значениями
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Количество мест, в которых нужно править - одинаковое. Если делать refactor -> introduce parameter, то работы не больше, чем при добавлении поля в кейскласс
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danslapman, извини, большинством оставим кейс классы. Если мы ошибаемся и это окажется лишним, то переделаем.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ashashev можно хотя-бы в отдельный файл модели вынести :-(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да, без проблем, в следующем PR вынесу в отдельный файл.
backend/mockingbird/src/main/scala/ru/tinkoff/tcb/mockingbird/dal2/mongo/HttpStubDAOImpl.scala
Outdated
Show resolved
Hide resolved
f5084ca
to
02724de
Compare
...xamples/src/main/scala/ru/tinkoff/tcb/mockingbird/edsl/interpreter/AsyncScalaTestSuite.scala
Outdated
Show resolved
Hide resolved
.../examples/src/main/scala/ru/tinkoff/tcb/mockingbird/edsl/interpreter/MarkdownGenerator.scala
Outdated
Show resolved
Hide resolved
9cf9175
to
4e809eb
Compare
1b69087
to
1dcd7f8
Compare
Add dal2.HttpStubDAO and impl for MongoDB that hides logic of building queries to Database. Added a DSL that lets describe an action sequence and then generates scalatest, executing it and markdown files containing curl commands for these actions.
1dcd7f8
to
99b54ca
Compare
LGTM |
Added new HttpStubDAO. It's first step in adding support of using a different database from MongoDB. New data access layer should hide building of databases queries.