Статьи

Surfacing Hidden Design: в поисках лучшей альтернативы взаимосвязанным изменяемым полям

Январь 25, 2019

Что лучше: группа изменяемых логических полей и методов, действующих на них, или явное выражение отдельных состояний и переходов между ними? Давайте рассмотрим пример из симуляции прогрессирования многостадийной инфекции.

1. Дизайн, скрытый в примитивных изменчивых полях и методах

Следующий класс с несколькими взаимосвязанными изменяемыми (общедоступными) полями, которые я должен был заполнить методами для переходов между их состояниями, сделал меня по-настоящему непростым (var — изменяемое поле, val — неизменяемый):

01
02
03
04
05
06
07
08
09
10
class Person (val id: Int) {
   
  var infected = false
  var sick = false
  var immune = false
  var dead = false
 
  // to complete with simulation logic
  // [infection progression]
}

Поля хранят состояние прогресса инфекции. Они зависят друг от друга — например, когда больна , человек также должен быть заражен , в то время как когда она мертва , она не должна быть иммунной .

Прогрессирование инфекции: здоровое -> инфицированное и заразное с вероятностью 40% (но пока нездорово) -> на 6-й день заметно больное -> на 14-й день умирает с вероятностью 25% -> если не умерло становится иммунным в 16-й день — не заметно болен, но все еще заразен -> здоров в 18-й день.

Проблема, связанная с сохранением состояния в куче полей, заключается в том, что нет явного выражения этих правил, и что оно открывает для дефектов, таких как присвоение больному значения « истина», в то время как забывается устанавливать и зараженное . Это также трудно понять. Вы можете изучить правила, изучив методы, которые изменяют поля, но это требует больших усилий, нелегко различить случайные детали реализации и намеренный дизайн, и этот код не предотвращает ошибки, подобные описанному.

Плюсы:

  • Знакомый
  • Легко оформить

Минусы:

  • Правила распространения заражения не ясны из кода (ну, это было бы не так, даже если бы методы были фактически показаны), то есть он плохо передает понятия и правила предметной области.
  • Значения нескольких полей должны быть синхронизированы, не гарантируя, что это приводит к дефектам
  • Приводит к спагетти-коду

2. Явное и недопустимое выражение состояний и переходов

Я бы предпочел сделать правила центральной частью проекта, а не основывать их на деталях реализации того, как мы сохраняем состояние (т. Е. На множестве логических значений для управления). Другими словами, я хочу раскрыть дизайн, скрытый в этом. Я хочу иметь четкое представление о состояниях — Здоровый, Инкубационный, Больной, Мертвый, Иммунный — с четкими переходами между ними, и я хочу, чтобы эти состояния явно несут вторичную информацию о том, заразен ли человек или нет, и является ли это видимым или нет.

Один способ выразить этот дизайн явно это:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
def randomBelow = ...
 
/** When should the health state change and what to */
class HealthChange(val time: Int, val newHealth: Health) {
  def immediate = (time == 0)
}
/** The root of the health hierarchy; the health (i.e. infection) evolves in stages */
sealed abstract class Health(val infectious: Boolean, val visiblyInfectious: Boolean) {
  def evolve(): Option[HealthChange] // returns None if no further change possibly/expected
}
/** In some stages the person is infected but it isn't visible*/
sealed abstract class CovertlyInfected extends Health(infectious = true, visiblyInfectious = false) {}
/** In other stages the person is infected and it is clearly visible*/
sealed abstract class VisiblyInfected extends Health(infectious = true, visiblyInfectious = true) {}
 
case object HEALTHY extends Health(infectious = false, visiblyInfectious = false) {
  def evolve() = // 40% chance of getting infected, triggered on meeting an infected person
    if (randomBelow(101) <= 40/*%*/)
      Some(new HealthChange(0, INCUBATIOUS))
    else
      None // no change, stays healthy
}
case object INCUBATIOUS extends CovertlyInfected {
  def evolve() = // After 6 days if infection without visible effects becomes sick
    Some(new HealthChange(6, SICK))
}
case object SICK extends VisiblyInfected {
  def evolve() = // die with 25% on day 14 or stays sick for 2 more days, then immune
    if (randomBelow(101) <= 25/*%*/)
        Some(new HealthChange(14 - 6, DEAD))
      else
        Some(new HealthChange(16 - 6, IMMUNE))
}
case object DEAD extends VisiblyInfected {
  def evolve() = None // Dead people stay dead
}
/** The symptoms have disappeared but the person can still infect others for 2 more days */
case object IMMUNE extends CovertlyInfected {
  def evolve() =
    Some(new HealthChange(18 - 16, HEALTHY))
}
 
class Person (val id: Int, var health: Health = HEALTHY) {
  def tryInfect() { // upon meeting an infected person
    if (health != HEALTHY) throw new IllegalStateException("Only healthy can be infected")
    health.evolve().foreach(
      healthChng => setHealth(healthChng.newHealth) // infection happens immediately
    )
  }
  /** Set the new health stage and schedule the next health change, if any */
  def setHealth(h: Health) {
    this.health = h
    h.evolve().foreach(hNext => {
      if (hNext.immediate)
        setHealth(hNext.newHealth)
      else
        afterDelay(hNext.time) {setHealth(hNext.newHealth)}
    })
  }
}

Pros

  • Правила и этапы заражения теперь явные и первоклассные члены кода; Доменно-управляемый дизайн на практике
  • Переходы между состояниями ясны, явны, и мы не можем перевести человека в недопустимое состояние (при условии, что мы правильно определили переходы)
  • Нам больше не нужно синхронизировать состояние нескольких переменных

Cons

  • Код, вероятно, длиннее, чем набор полей bool и методов для перехода между их состояниями.
  • Это может показаться сложным, потому что вместо одного класса и нескольких методов мы внезапно имеем иерархию классов; но на самом деле это предотвращает сложность исходного кода спагетти, поэтому, хотя и не «легкое» для понимания, оно «простое», согласно Р. Хики

Вывод

Я часто сталкиваюсь с подобным кодом, особенно в старых унаследованных приложениях, которые были разработаны в соответствии с меняющимися бизнес-потребностями с уделением основного внимания «функциям» без соответствующего обновления / рефакторинга лежащего в основе проекта. Честно говоря, это ад. Низкоуровневый код, работающий с несколькими взаимозависимыми полями (в классе, который, вероятно, также имеет несколько несвязанных полей или полей, которые зависят от них только в определенных случаях использования), является раем для дефектов, которые можно скрыть и размножить. И это трудно понять, так как дизайн — правила, концепции и намерения — не были сделаны явными (пока). И трудно понять, значит трудно изменить.

Поэтому важно регулярно просматривать ваш код и выявлять скрытый в нем дизайн, скрывая детали реализации низкого уровня и делая ключевые понятия, состояния, переходы, правила и т. Д. Первоклассными членами базы кода, чтобы читать код ощущается как общение и обучение, а не как археология.

Обновления

Обновление 1

Код на основе Enum и Java от Jerrinot, включенный сюда для удобства чтения:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
public enum Health {
    HEALTHY(false, false, false, false),
    INCUBATIOUS(true, false, false, false),
    SICK(true...),
    DEAD(....);
 
    private boolean infected;
    private boolean sick;
    private boolean immune;
    private boolean dead;
 
    private Health(boolean infected, boolean sick, boolean immune, boolean dead) {
        this.infected  = infected;
        this.sick = sick;
        this.immune = immune;
        this.dead = dead;
    }
 
    public boolean isInfected() {
        return infected;
    }
 
    public boolean isSick() {
        return sick;
    }
 
    public Health evolve() {
        switch (this) {
           case HEALTHY:
               return computeProbabability..() ? INCUBATIOUS : HEALTHY;
[....]
        }
    }
}

Обновление 2

Версия Джеррино в Скале:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
sealed class Health(val infectious: Boolean, val visiblyInfectious: Boolean) {
 
  def evolve(implicit randomGenerator: RandomIntGenerator, config: MySimConfig): Option[HealthChange] =
    this match {
      case Healthy =>
        if (randomGenerator.randomBelow(101) <= config.transmissibilityPct)
          Some(new HealthChange(0, Incubatious))
        else None
      case Incubatious =>
        Some(new HealthChange(6, Sick))
      case Sick =>
        if (randomGenerator.randomBelow(101) <= config.mortalityPct)
          Some(new HealthChange(14 - 6, Dead))
        else None
      case Dead =>
        None
      case Immune =>
        Some(new HealthChange(18 - 16, Healthy))
      case Vaccinated =>
        None       
    }
}
//                                 infected?  visibly?
object Healthy extends      Health(false,     false)
object Incubatious extends  Health(true,      false)
object Sick extends         Health(true,      true)
object Dead extends         Health(true,      true)
object Immune extends       Health(true,      false)
object Vaccinated extends   Health(false,     false)

«Примитивная» версия с почти полным кодом, исключая информацию о том, что происходит, когда:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
// P.S.: Excuse the underscores ...
class Person (val id: Int) {
 
  // Simplified primitive version, assuming the information when should the evolution
  // happen is handled somewhere else
  def evolveHealth(): Unit = {
    if (!_infected)
      if (randomGenerator.randomBelow(101) <= config.transmissibilityPct)
        this._infected = true
    if (_infected && !_sick && !_immune)
      this._sick = true
    if (_sick)
      if (randomGenerator.randomBelow(101) <= config.mortalityPct)
        this._dead = true
      else
        this._unDead = true // did not die but still sick
    if (_unDead)
      this._immune = true
      this._sick = true
    if (_immune)
      this._immune = false
      this._infected = false
  }
   
  private var _unDead = false
  private var _infected = false
  private var _sick = false
  private var _immune = false
  private var _dead = false
}

34
Share: