Как мне кажется проблемы с читабельностью имеют две основные причины.
- 3 возможных состояния объекта закодированы в 3 аттрибутах. При этом возможны заведомо бессмысленные состояния (doneCnt != res.size() или ex != null && doneCnt == totCnt).
- Методы fail, add, listen судя по коду предпринимают какие-то действия в слепую, иногда проверяя лишь часть переменных. Как следствие о корректности кода нельзя судить по отдельным фрагментам - нужно думать о поведении всех методов сразу.
Суть выражена в переходе от набора из нескольких аттрибутов к одному но с более сложной структурой.
Было:
private var doneCnt = 0 private var ex: Exception = null private val res = new ArrayList[T](totCnt)Cтало:
private var res: Either[util.ArrayList[T], Exception] = Left(new util.ArrayList[T](totCnt))Помимо экономии пары строк мы добавили немного семантики изменяемому состоянию объектов. Теперь видно что наличие исключения и списка результатов - взаимоисключающие состояния. Но есть ли ещё какие-то скрытые значения в состоянии списка? С одной стороны можно бы это описать в комментариях, но они как известно не проверяются компилятором. Так что я попробовал сделать так чтобы она стала ясна из прочтения первого-же метода.
Было:
def add(item: T) {
lock.lock()
try {
res.add(item)
doneCnt = doneCnt + 1
if (doneCnt == totCnt) {
done.signalAll()
for (lsnr <- succLsnrs)
lsnr(res)
}
}
finally {
lock.unlock()
}
}
Cтало:def add(item: T) {
lock.lock()
try {
res match {
case Left(list) if list.size < totCnt => {
list.add(item)
if (list.size == totCnt) {
done.signalAll()
for (lsnr <- succLsnrs)
lsnr(list)
}
}
case _ => return
}
}
finally {
lock.unlock()
}
}
Смотрим метод add, из него видно что добавления результатов разрешены пока количество элементов в списке меньше totCnt. Здесь уже вырисовывается точная семантика для аттрибута res.- Right(exception) - сбор результатов провалился.
- Left(list) && list.size < totCnt - сбор результатов всё ещё в процессе.
- Left(list) && list.size == totCnt - сбор результатов завершён их все можно получить.
Метод fail тоже неплохо прибавил в выразительности - теперь оба add и fail явно переводят объект из одного состояния в другое. Надо признать что новая версия ведёт себя не точно так-же как старая, новый подход сделал более строгое поведение и более лёгким. add больше не принимает результаты сверх заданного числа, а fail - не изменяет состояние завершённого объекта.
Было:
def fail(e: Exception) {
lock.lock()
try {
ex = e
done.signalAll()
for (lsnr <- failLsnrs)
lsnr(ex)
}
finally {
lock.unlock()
}
}
Cтало:def fail(e: Exception) {
lock.lock()
try {
res match {
case Left(list) if list.size < totCnt => {
res = Right(ex)
done.signalAll()
for (lsnr <- failLsnrs)
lsnr(ex)
}
case _ => return
}
}
finally {
lock.unlock()
}
}
Существенно улучшилась и структура метода get - теперь он сначала дожидается одного из завершённых состояний, потом предпринимает некие действия. Раньше он вынужден был выполнять две по-сути параллельные проверки во время ожидания и по-разному из него выходить.Было:
def get: JavaCollection[T] = {
lock.lock()
try {
while (doneCnt != totCnt) {
if (ex != null)
throw ex
done.await()
}
res
}
finally {
lock.unlock()
}
}
Cтало:def get: JavaCollection[T] = {
lock.lock()
try {
while (res.left.toOption.exists(_.size < totCnt))
done.await()
res match {
case Left(list) => list
case Right(ex) => throw ex
}
}
finally {
lock.unlock()
}
}
Метод listen в принципе изменился так же как get, так что я не буду приводить его код. Вот окончательная версия.Я вижу следующие явные улучшения:
- Вместо 3х скоординированных значений - одно, содержащее результат. Состояний по-прежнему три, но способ их кодирования радикально упростился.
- Методы add, fail, listen изменили форму. Раньше они более а в основном менее явно предполагали некую историю изменений и обновляли состояние на основе этих предположений. Теперь методы инспектируют текущее состояние, и устанавливают новое на его основе. Практически конечный автомат.
- Метод get сначала неявно ожидал нужного состояния одной из двух переменных (ex и size), "произошло исключение или количество результатов сравнялось с ожидаемым". Теперь этот метод явно ожидает пока нарушится одно условие, "существует неполный список результатов". Точек принятия решения в методе изначально было две, стала одна.
Одно общее следствие: класс стало возможно анализировать и читать более мелкими шагами, по одному методу. При этом пространство возможных состояний уменьшилось лишь немного, изменилось лишь их представление и интерпретация.
