Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the state-based counter prototype #138

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

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.

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

@@ -72,6 +71,51 @@ internal fun lastWriterWinsResolve(resolveAttrName: (String) -> Attr<Any>?): (Li
}
}

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()
  // ...
}

@@ -72,6 +71,51 @@ internal fun lastWriterWinsResolve(resolveAttrName: (String) -> Attr<Any>?): (Li
}
}

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 было

qbit-core/src/commonTest/kotlin/qbit/FunTest.kt Outdated Show resolved Hide resolved
@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