Как написать меньше кода для длинного оператора if-else в Scala (обзор кода)

Question

У меня есть следующий код:

  def updateProgrammcounter(element: Name) {
    val level = findSecurityLevelOfNameInSymboltable(element.toString.trim)
    var pc = new HashSet[String]
    println("LEVEL: " + level)
    if (pc.isEmpty && level != "N") {
      // the pc is empty so add the level
      pc += level
    } else if (pc.contains("H") && level == "L") {
      // giving high information to low is not allowed
      errorReject()
    } else if (pc.contains("L") && level == "L") {
      // nothing to change because the pc is the same
      println("okay")
    } else if (pc.contains("H") && level == "H") {
      println("okay")
    } else if (pc.contains("L") && level == "H")
      // we have to raise the pc
    pc += level
    else if (level == "N") {
      println("We have a not valid operation!")
    } else {
      println("Something went terribly wrong!")
    }
  }

Вы видите какой-то уровень улучшения, который я могу сделать, чтобы этот большой кусок кода выглядел просто лучше.Спасибо за совет, Матиас.

Answer 1

Один из распространенных приемов состоит в том, чтобы заключить два значения (pc и level) в кортеж, давая вам одну сущность, с которой вы можете сопоставить шаблон:

(pc,level) match {
  case (a,b) if condition => ...
  case _ => ...
}

Проверка pc.isEmpty сразу после создания пустого pc предполагает, что вы обрезали код для stackoverflow, поэтому этот подход может вам здесь не помочь. Но я все равно попробую:

def updateProgrammcounter(element: Name) {
  val level = findSecurityLevelOfNameInSymboltable(element.toString.trim)
  var pc = new HashSet[String]
  println("LEVEL: " + level)

  (pc, level) match {
    case (pc, level) if pc.isEmpty && level != "N" => pc += level
    case (pc, "L") if pc.contains("H") => errorReject()
    case (pc, "L") if pc.contains("L") => println("okay")
    case (pc, "H") if pc.contains("H") => println("okay")
    case (pc, "H") if pc.contains("L") => pc += level
    case (_,  "N") => println("We have a not valid operation!")
    case _ => println("Something went terribly wrong!")
  }
}

Это, очевидно, отсутствие, поскольку половина этих условий невозможна, но является буквальным переводом кода, который вы изначально предоставили. Я также недоволен побочными эффектами и мутациями, но не хотел слишком сильно меняться за один присест!

Если вы сможете добавить обратно в удаленный код или дать лучшее представление о том, чего вы пытаетесь достичь, я с радостью обновлю свой ответ:)

ОБНОВЛЕНИЕ По подсказке Даниила, вот следующий шаг; перестановка и оптимизация корпусов:

(pc, level) match {
  case (_,  "N") => println("We have a not valid operation!")
  case (pc, level) if pc.isEmpty => pc += level
  case (pc, level) if pc.contains(level) => println("okay")
  case (pc, "L") if pc.contains("H") => errorReject()
  case (pc, "H") if pc.contains("L") => pc += level
  case _ => println("Something went terribly wrong!")
}

Answer 2

Вы, конечно, можете переставить это на уровни, затем pc:

def updateProgrammcounter(element: Name) {
    val level = findSecurityLevelOfNameInSymboltable(element.toString.trim)
    var pc = new HashSet[String]
    println("LEVEL: " + level)
    if (level == "L") {
    } else if (level == "H") {
        // All the "H" stuff in here
    } else {
       // Put the "N" case at the top here - it looks like your fall through default.
    }
  }

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

Это может указывать на то, что у вас могут быть объекты обновлений L и H, которые вы можете найти, используя уровень.

Вы также можете объединить уровень и компьютер в ключ на карте и посмотреть, что бы вы хотели сделать.

Ваш инстинкт хорош: когда вы видите длинные if / then / else, вы должны подумать о полиморфизме или поисках, чтобы упростить его. Подумайте, как можно изменить поток, добавляя объекты, а не переписывая код (принцип открытия / закрытия).