Skip to content

Conversation

@ajani2001
Copy link

No description provided.

Copy link
Owner

@d-r-q d-r-q left a comment

Choose a reason for hiding this comment

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

Ну и прошлые комменты надо править

}
}

internal fun crdtResolve(
Copy link
Owner

Choose a reason for hiding this comment

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

Эммм, это очень странно.
Зачем дублировать логику lastWriterWinsResolve? А тесты не дублировать?

// temporary dirty hack until crdt counter or custom resolution strategy support is implemented
attr == Instances.nextEid -> listOf((eavsFromA + eavsFromB).maxByOrNull { it.eav.value as Int }!!.eav)
attr.list -> (eavsFromA + eavsFromB).map { it.eav }.distinct()
DataType.ofCode(attr.type)!!.isCounter() -> {
Copy link
Owner

Choose a reason for hiding this comment

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

Симетрия и постоянство уровней абстракции - очень важные свойства кода.
Почитай Implementation Patterns (Symmetry) и Clean Code (G34: Functions Should Descend Only One Level of Abstraction)
А эту портянку вынеси в отдельный метод

Eav(
eavsFromA[0].eav.gid,
eavsFromA[0].eav.attr,
if (previous is Byte && latestFromA is Byte && latestFromB is Byte) latestFromA + latestFromB - previous
Copy link
Owner

Choose a reason for hiding this comment

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

Это тоже лучше утащить в

operator fun Number.plus(another: Number) = when (this) {
  is Int -> this + another.toInt()
  // ...
}

}

internal fun crdtResolve(
resolveEntity: (Gid) -> StoredEntity?,
Copy link
Owner

Choose a reason for hiding this comment

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

resolveBaseEntity

И ваще лучше коммент к методу написать. Как писать хоршие комменты можешь почитать в Philosophy of Software Design и вроде том же Clean Code было

@d-r-q
Copy link
Owner

d-r-q commented Apr 24, 2022

А что с правками по этому МРу?

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.

2 participants