Рефакторинг/компоновка функциональных Scala

Ответ 1

Тривиальная попытка первой попытки убрать его - просто удалить ведущий Console. конечный ; и явный тип :String - все это не нужно - добавить некоторые отступы и импортировать io.Source:

import io.Source
println(
  Source.fromFile("names.txt").getLines.mkString.split(",").map{
    x => x.slice(1, x.length -1)
  }.sortBy {x => x}.zipWithIndex.map{
    t =>{ (t._2 +1)*(t._1.map{_.toChar - "A"(0).toChar + 1}.sum)}
  }.sum
)

Следующий шаг - немного почистить его, использовать сопоставление шаблонов при сопоставлении списка кортежей и identity вместо x=>x. toChar также не требуется для символов, а одинарные кавычки могут использоваться для представления символьных литералов.

import io.Source
println(
  Source.fromFile("names.txt").getLines.mkString.split(",").map {
    x => x.slice(1, x.length -1)
  }.sortBy(identity).zipWithIndex.map {
    case (v, idx) =>{ (idx+1)*(v.map{_ - 'A' + 1}.sum)}
  }.sum
)

Еще несколько изменений также помогают сделать код намного яснее:

import io.Source
println(
  Source.fromFile("names.txt").getLines.mkString.split(",")
  .map { _.stripPrefix("\"").stripSuffix("\"") }
  .sortBy(identity)
  .map { _.map{_ - 'A' + 1}.sum }
  .zipWithIndex
  .map { case (v, idx) => (idx+1) * v }
  .sum
)

Следующий шаг, чтобы сделать его более "функциональным", - это разбить его на "функции" (подлый, да?). В идеале каждая функция будет иметь имя, четко выражающее его назначение, и оно будет коротким (цель состоит в том, чтобы это было одно выражение, поэтому скобки не требуются):

import io.Source

def unquote(s:String) = s.stripPrefix("\"").stripSuffix("\"")

def wordsFrom(fname:String) =
  Source.fromFile(fname).getLines.mkString.split(",").map(unquote)

def letterPos(c:Char) = c - 'A' + 1

println(
  wordsFrom("names.txt")
  .sortBy(identity)
  .map { _.map(letterPos).sum }
  .zipWithIndex
  .map { case (v, idx) => (idx+1) * v }
  .sum
)

wordsFrom является очевидным 1-лайнером, но я разделил его для упрощения форматирования на stackOverflow

Ответ 2

Вот что я считаю лучшим способом выложить:

Console.println(
    io.Source.fromFile("names.txt")
    .getLines.mkString.split(",")
    .map{x:String => x.slice(1, x.length -1)}
    .sortBy { x => x}
    .zipWithIndex
    .map{t =>{ (t._2 +1)*(t._1.map{_.toChar - "A"(0).toChar + 1}.sum)}}
    .sum); 

Мне кажется, что в глубинах моего мозга есть какой-то алгоритм, который принимает решения о компрометации кода между горизонтальным и вертикальным пространством, но у меня нет прямого доступа, чтобы я мог его сформулировать.:)

Что касается введения имен, а не использования lambdas, я думаю, что я обычно делаю, если у меня возникает соблазн поставить короткий комментарий, описывающий цель кода, но хорошее имя идентификатора может сделать то же самое, тогда я могу одноразовая лямбда в именованную функцию, чтобы получить полезность для имени идентификатора. Строка с вызовами toChar - это только одна, которая выглядит как кандидат для меня. (Чтобы быть ясным, я бы включил (часть) лямбда внутри map, но сам вызов map.) Альтернативно, введение вертикальных пробелов дает вам место для зависания //comment, которое является альтернатива введению имени идентификатора.

(Отказ от ответственности: я не пишу Scala, поэтому, если что-то, что я говорю, конфликтует с соглашениями стилей, то игнорирую меня:), но я думаю, что многие из этих советов в основном-языковые-агностики.)

Ответ 3

Говоря строго о том, как форматировать этот код, без каких-либо структурных изменений, я бы сделал это следующим образом:

Console println (
  (
    io.Source
    fromFile "names.txt"
    getLines ()
    mkString ""
    split ","
    map (x => x.slice(1, x.length - 1))
    sortBy (x => x)
    zipWithIndex
  )
  map (t => (t._2 + 1) * (t._1 map (_.toChar - "A"(0).toChar + 1) sum) )
  sum
)

Или, возможно, чтобы обойти беспараметричные методы, я бы сделал это:

Console println (
  io.Source
  .fromFile("names.txt")
  .getLines
  .mkString
  .split(",")
  .map(x => x.slice(1, x.length - 1))
  .sortBy(x => x)
  .zipWithIndex
  .map(t => (t._2 + 1) * (t._1 map (_.toChar - "A"(0).toChar + 1) sum) )
  .sum
)

Обратите внимание, что есть много места для комментариев, но, вообще говоря, что обычно делается ясно. Люди, не привыкшие к нему, иногда могут потеряться на полпути, без переменных, чтобы отслеживать значение/тип преобразованное значение.

Теперь некоторые вещи, которые я буду делать по-другому:

println ( // instead of Console println
  Source // put import elsewhere
  .fromFile("names.txt")
  .mkString // Unless you want to get rid of /n, which is unnecessary here
  .split(",")
  .map(x => x.slice(1, x.length - 1))
  .sorted // instead of sortBy
  .zipWithIndex
  .map { // use { only for multiple statements and, as in this case, pattern matching
    case (c, index) => (index + 1) * (c map (_ - 'A' + 1) sum) // chars are chars
  }
  .sum
)

Я бы тоже не делал суммирования и умножения на том же шаге, поэтому:

  .sorted
  .map(_ map (_ - 'A' + 1) sum)
  .zipWithIndex
  .map { case (av, index) => av * (index + 1) }
  .sum

Наконец, мне не очень нравится изменение размера строки, поэтому я могу прибегнуть к регулярному выражению. Добавьте немного рефакторинга, и это то, что я, вероятно, напишу:

  import scala.io.Source
  def names = Source fromFile "names.txt" mkString

  def wordExtractor = """"(.*?)"""".r
  def words = for {
    m <- wordExtractor findAllIn names matchData
  } yield m group 1

  def alphabeticValue(s: String) = s map (_ - 'A' + 1) sum
  def wordsAV = words.toList.sorted map alphabeticValue

  def multByIndex(t: (Int, Int)) = t match {
    case (av, index) => av * (index + 1)
  }
  def wordsAVByIndex = wordsAV.zipWithIndex map multByIndex

  println(wordsAVByIndex.sum)

ИЗМЕНИТЬ

Следующим шагом будет переименование рефакторинга - выбор имен, которые лучше передают то, что делает каждая часть кода. Кэн предложил лучшие имена в комментариях, и я бы подгонял их для еще одного варианта (он также прекрасно показывает, насколько лучше именование улучшает читаемость).

import scala.io.Source
def rawData = Source fromFile "names.txt" mkString

// I'd rather write "match" than "m" in the next snippet, but
// the former is a keyword in Scala, so "m" has become more
// common in my code than "i". Also, make the return type of
// getWordsOf clear, because iterators can be tricky.
// Returning a list, however, makes a much less cleaner
// definition.

def wordExtractor = """"(.*?)"""".r
def getWordsOf(input: String): Iterator[String] = for {
  m <- wordExtractor findAllIn input matchData
} yield m group 1
def wordList = getWordsOf(rawData).toList

// I stole letterPosition from Kevin solution. There, I said it. :-)

def letterPosition(c: Char) = c.toUpper - 'A' + 1 // .toUpper isn't necessary
def alphabeticValueOfWord(word: String) = word map letterPosition sum
def alphabeticValues = wordList.sorted map alphabeticValueOfWord

// I don't like multAVByIndex, but I haven't decided on a better
// option yet. I'm not very fond of declaring methods that return
// functions either, but I find that better than explicitly handling
// tuples (oh, for the day tuples/arguments are unified!).

def multAVByIndex = (alphabeticValue: Int, index: Int) => 
  alphabeticValue * (index + 1)
def scores = alphabeticValues.zipWithIndex map multAVByIndex.tupled

println(scores sum)

Ответ 4

В дополнение к решению Кевина

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

Чтобы сделать код еще короче и чище, кажется, что выражение for может использоваться.

def inputString(inputFile: String) = io.Source.fromFile(inputFile).getLines.mkString

def inputWords(input: String) = input.split("[,\"]").filter("" != )

Console.println {
    (for { (s, idx) <- inputWords(inputString("names.txt")).sorted.zipWithIndex }
        yield s.map(_ - 'A' + 1).sum * (idx + 1)).sum
}

Часть s.map(_- 'A' + 1) может быть добавлена ​​в функцию, например wordSum, если вы хотите, чтобы она была более отчетливой.