Skip to content

Conversation

@Robalim
Copy link
Contributor

@Robalim Robalim commented Mar 19, 2024

override fun checkTargeting(data: TargetingData): Boolean {
val userVisitCount = MindboxPreferences.userVisitCount.toLong()
return when (kind) {
KindVisit.GTE -> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Зачем скобки ставить? мне нравится больше когда в одну строку
KindVisit.GTE -> value >= userVisitCount

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Я обычно всегда ставлю, на случай, что если придется что-то дописывать, то не пришлось бы их ставить потом. А у нас нет нигде какого-то код стайла, чтобы посмотреть как лучше писать

is TreeTargetingDto.VisitNodeDto -> {
!targeting.type.isNullOrBlank() && targeting.kind.equalsAny(
GREATER_OR_EQUALS, LOWER_OR_EQUALS, EQUALS, NOT_EQUALS
) && (targeting.value?.let { it > 0 } == true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

А 0 не может быть?

Copy link
Collaborator

Choose a reason for hiding this comment

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

согласен что > 0
"value": 1 // required long > 0

}

@Test
fun `validate targeting dto is visit node and its valid with gte`() {
Copy link
Collaborator

@sergeysozinov sergeysozinov Mar 20, 2024

Choose a reason for hiding this comment

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

Здесь бы получше было что-то такое validate targeting dto return true when is visit node and its valid with gte, но здесь сразу проверка, поэтому тест и так хорошо читается

@Robalim Robalim merged commit d7e4cbe into develop Mar 20, 2024
@Robalim Robalim deleted the feature/MBX-3211 branch March 20, 2024 09:48
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.

4 participants