Refactoring/bố trí chức năng Scala

Question

này lót ...

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); 

... là giải pháp của tôi để Project Euler problem 22. Dường như nó hoạt động, và nó được viết theo phong cách chức năng (cố gắng của tôi).

Ví dụ này hơi khắc nghiệt, nhưng câu hỏi của tôi hơi chung chung hơn một chút - bạn thích viết/định dạng/nhận xét mã kiểu chức năng như thế nào? Cách tiếp cận chức năng dường như khuyến khích một chuỗi các cuộc gọi phương thức, mà tôi tìm thấy có thể không đọc được, và cũng để lại không rõ ràng để đưa ra nhận xét.

Ngoài ra, khi tôi viết mã thủ tục, tôi thấy tôi viết từng phương pháp nhỏ với một mục đích và với các tên có ý nghĩa. Khi tôi viết mã chức năng, tôi dường như đang phát triển một thói quen tạo ra các dòng giống như ở trên, ở đâu (đối với tôi) ý nghĩa khó giải mã - và cũng có thể khó tính lại các tính toán riêng lẻ ở nơi khác. Rất nhiều ví dụ mã chức năng tôi thấy trên web tương tự như vậy (cho tôi) tối nghĩa.

Tôi nên làm gì? Viết các hàm nhỏ cho mỗi phần của tính toán với các tên có ý nghĩa trong ngữ cảnh hiện tại? (ngay cả khi chúng ít hơn một wrapper cho bản đồ, nói?)

Ví dụ tôi đã đưa ra, cách tốt hơn để viết nó là gì, và trình bày nó?

(Giống như tất cả các câu hỏi phong cách, cái này là chủ quan Không có lý do nó sẽ nhận được tranh cãi, mặc dù.!)

Answer 1

Một đầu nỗ lực tầm thường tại làm sạch nó lên là chỉ cần loại bỏ các hàng đầu Console. các dấu ; và rõ ràng :String loại - tất cả trong số đó là không cần thiết - thêm một số thụt đầu dòng và nhập khẩu 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 
) 

bước tiếp theo là làm sạch nó một chút, sử dụng mẫu phù hợp khi lập bản đồ trên danh sách các bộ dữ liệu nd identity thay vì x=>x. toChar cũng không cần thiết đối với các ký tự và các dấu nháy đơn có thể được sử dụng để biểu diễn các ký tự chữ.

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 
) 

Một vài thay đổi nhiều hơn cũng giúp làm cho mục đích của mã xa rõ ràng hơn:

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 
) 

Bước tiếp theo, để làm cho nó nhiều hơn "chức năng", là để phá vỡ nó thành "chức năng" (lén lút, huh?). Lý tưởng nhất là mỗi chức năng sẽ có một tên thể hiện rõ mục đích của nó, và nó sẽ được ngắn (nhằm mục đích cho nó trở thành một biểu thức duy nhất, vì vậy niềng răng không bắt buộc):

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 là một rõ ràng 1-liner, nhưng tôi chia nó để định dạng dễ dàng hơn trên Stackoverflow

Answer 2

Dưới đây là những gì tôi nghĩ có thể là một cách tốt hơn để đẻ nó ra:

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); 

Tôi cảm thấy sâu thẳm trong bộ não của mình, có một số thuật toán đưa ra quyết định về giao dịch bố cục mã giữa không gian ngang và dọc, nhưng dường như tôi không có quyền truy cập trực tiếp để cho phép tôi nói rõ. :)

Về việc giới thiệu tên thay vì sử dụng lambdas, tôi nghĩ rằng những gì tôi thường làm là, nếu tôi bị cám dỗ để đặt một nhận xét ngắn mô tả ý định của mã, nhưng một tên nhận dạng tốt có thể làm như vậy, sau đó tôi có thể yếu tố lambda một lần vào một hàm được đặt tên để có được lợi ích dễ đọc của tên định danh. Đường dây có các cuộc gọi toChar là cuộc gọi duy nhất ở trên trông giống như một ứng cử viên đối với tôi. (Để được rõ ràng, tôi muốn yếu tố (một phần của) lambda bên trong các map, nhưng các cuộc gọi map chính nó.) Ngoài ra, việc giới thiệu khoảng trắng dọc cung cấp cho bạn một nơi để treo một //comment đó là một thay thế để giới thiệu một tên định danh .

(Disclaimer:. Tôi không viết Scala, vì vậy nếu bất cứ điều gì tôi nói mâu thuẫn với quy ước phong cách, sau đó bỏ qua cho tôi :), nhưng tôi tưởng tượng rất nhiều lời khuyên này chủ yếu là ngôn ngữ-agnostic)

Answer 3

Ngoài giải pháp của Trịnh Gia Dĩnh,

chính là để phân chia các chức năng rõ ràng và gọn gàng, lấy trong việc xem xét việc tái sử dụng và khả năng đọc.

Để làm cho mã thậm chí ngắn hơn và sạch hơn, có vẻ như biểu thức cho có thể được sử dụng.

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 
} 

Phần s.map (_- 'A' + 1) có thể được thêm vào hàm, nói wordSum, nếu bạn muốn nó khác biệt hơn.

Answer 4

Phát biểu Nghiêm làm thế nào để định dạng mã mà, mà không thực hiện bất kỳ thay đổi cấu trúc, tôi muốn làm điều đó như thế này:

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 
) 

Hoặc, có lẽ, để có được xung quanh phương pháp parameterless, tôi muốn làm điều này:

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 
) 

Lưu ý rằng có nhiều không gian cho ý kiến, nhưng, nói chung, những gì đang được thực hiện thường rõ ràng. Những người không quen với nó đôi khi có thể bị mất nửa chừng, không có biến để theo dõi ý nghĩa/loại của giá trị được chuyển đổi .

Bây giờ, một số những điều tôi muốn làm khác đi là:

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 
) 

Tôi cũng không làm tổng và nhân lên trong các bước tương tự, vì vậy:

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

Cuối cùng, tôi don' Tôi thích thay đổi kích thước chuỗi, vì vậy tôi có thể sử dụng regex để thay thế. Thêm một chút tái cấu trúc, và đây là điều mà tôi có thể viết:

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) 

EDIT

Bước tiếp theo sẽ là một refactoring đổi tên - chọn tên mà truyền đạt tốt hơn những gì mỗi một phần của mã đang làm. Ken đề xuất tên tốt hơn trong các ý kiến, và tôi thích hợp cho họ cho một biến thể hơn (nó cũng giới thiệu độc đáo bao nhiêu tốt hơn đặt tên cải thiện khả năng đọc).

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's 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)