2010-06-17 25 views
5

Tôi là một Scala n00b (nhưng có kinh nghiệm với các ngôn ngữ khác) và đang học ngôn ngữ như tôi tìm thấy thời gian - rất thích thú với nó cho đến nay!Phê bình mã Scala của tôi

Thông thường khi học một ngôn ngữ mới, điều đầu tiên tôi làm là thực hiện Conway's Game of Life, vì nó chỉ đủ phức tạp để mang lại ý thức tốt về ngôn ngữ, nhưng đủ nhỏ để có thể bắt kịp trong vài giờ (hầu hết trong số đó là chi tiêu đấu vật với cú pháp).

Anyhoo, đã trải qua bài tập này với Scala Tôi đã hy vọng các bậc thầy Scala ra có thể xem xét mã tôi đã kết thúc và cung cấp phản hồi về nó. Tôi đang theo đuổi bất cứ điều gì - cải tiến về thuật toán (đặc biệt là các giải pháp đồng thời!), Cải tiến phong cách, các API thay thế hoặc cấu trúc ngôn ngữ, ghê tởm ở độ dài tên chức năng của tôi - bất kỳ phản hồi nào bạn có, tôi rất muốn nghe nó!

Bạn sẽ có thể chạy tập lệnh sau thông qua scala GameOfLife.scala - theo mặc định nó sẽ chạy một bảng 20x20 với một tàu lượn đơn trên đó - vui lòng thử nghiệm.

// CONWAY'S GAME OF LIFE (SCALA) 
abstract class GameOfLifeBoard(val aliveCells : Set[Tuple2[Int, Int]]) 
{ 
    // Executes a "time tick" - returns a new board containing the next generation 
    def tick : GameOfLifeBoard 

    // Is the board empty? 
    def empty : Boolean = aliveCells.size == 0 

    // Is the given cell alive? 
    protected def alive(cell : Tuple2[Int, Int]) : Boolean = aliveCells contains cell 

    // Is the given cell dead? 
    protected def dead(cell : Tuple2[Int, Int]) : Boolean = !alive(cell) 

} 


class InfiniteGameOfLifeBoard(aliveCells : Set[Tuple2[Int, Int]]) 
    extends GameOfLifeBoard(aliveCells) 
{ 
    // Executes a "time tick" - returns a new board containing the next generation 
    override def tick : GameOfLifeBoard = new InfiniteGameOfLifeBoard(nextGeneration) 

    // The next generation of this board 
    protected def nextGeneration : Set[Tuple2[Int, Int]] = aliveCells flatMap neighbours filter shouldCellLiveInNextGeneration 

    // Should the given cell should live in the next generation? 
    protected def shouldCellLiveInNextGeneration(cell : Tuple2[Int, Int]) : Boolean = (alive(cell) && (numberOfAliveNeighbours(cell) == 2 || numberOfAliveNeighbours(cell) == 3)) || 
                        (dead(cell) && numberOfAliveNeighbours(cell) == 3) 

    // The number of alive neighbours for the given cell 
    protected def numberOfAliveNeighbours(cell : Tuple2[Int, Int]) : Int = aliveNeighbours(cell) size 

    // Returns the alive neighbours for the given cell 
    protected def aliveNeighbours(cell : Tuple2[Int, Int]) : Set[Tuple2[Int, Int]] = aliveCells intersect neighbours(cell) 

    // Returns the coordinates of all of the neighbouring cells of the given cell 
    protected def neighbours(cell : Tuple2[Int, Int]) : Set[Tuple2[Int, Int]] = Set((cell._1-1, cell._2-1), (cell._1, cell._2-1), (cell._1+1, cell._2-1), 
                        (cell._1-1, cell._2),       (cell._1+1, cell._2), 
                        (cell._1-1, cell._2+1), (cell._1, cell._2+1), (cell._1+1, cell._2+1)) 

    // Information on where the currently live cells are 
    protected def xVals = aliveCells map { cell => cell._1 } 
    protected def xMin = (xVals reduceLeft (_ min _)) - 1 
    protected def xMax = (xVals reduceLeft (_ max _)) + 1 
    protected def xRange = xMin until xMax + 1 

    protected def yVals = aliveCells map { cell => cell._2 } 
    protected def yMin = (yVals reduceLeft (_ min _)) - 1 
    protected def yMax = (yVals reduceLeft (_ max _)) + 1 
    protected def yRange = yMin until yMax + 1 


    // Returns a simple graphical representation of this board 
    override def toString : String = 
    { 
    var result = "" 

    for (y <- yRange) 
    { 
     for (x <- xRange) 
     { 
     if (alive (x,y)) result += "# " 
     else result += ". " 
     } 

     result += "\n" 
    } 

    result 
    } 

    // Equality stuff 
    override def equals(other : Any) : Boolean = 
    { 
    other match 
    { 
     case that : InfiniteGameOfLifeBoard => (that canEqual this) && 
              that.aliveCells == this.aliveCells 
     case _ => false 
    } 
    } 

    def canEqual(other : Any) : Boolean = other.isInstanceOf[InfiniteGameOfLifeBoard] 

    override def hashCode = aliveCells.hashCode 

} 


class FiniteGameOfLifeBoard(val boardWidth : Int, val boardHeight : Int, aliveCells : Set[Tuple2[Int, Int]]) 
    extends InfiniteGameOfLifeBoard(aliveCells) 
{ 
    override def tick : GameOfLifeBoard = new FiniteGameOfLifeBoard(boardWidth, boardHeight, nextGeneration) 

    // Returns the coordinates of all of the neighbouring cells of the given cell 
    override protected def neighbours(cell : Tuple2[Int, Int]) : Set[Tuple2[Int, Int]] = super.neighbours(cell) filter { cell => cell._1 >= 0 && cell._1 < boardWidth && 
                                   cell._2 >= 0 && cell._2 < boardHeight } 

    // Information on where the currently live cells are 
    override protected def xRange = 0 until boardWidth 
    override protected def yRange = 0 until boardHeight 

    // Equality stuff 
    override def equals(other : Any) : Boolean = 
    { 
    other match 
    { 
     case that : FiniteGameOfLifeBoard => (that canEqual this) && 
              that.boardWidth == this.boardWidth && 
              that.boardHeight == this.boardHeight && 
              that.aliveCells == this.aliveCells 
     case _ => false 
    } 
    } 

    override def canEqual(other : Any) : Boolean = other.isInstanceOf[FiniteGameOfLifeBoard] 

    override def hashCode : Int = 
    { 
    41 * (
     41 * (
     41 + super.hashCode 
    ) + boardHeight.hashCode 
    ) + boardWidth.hashCode 
    } 

} 


class GameOfLife(initialBoard: GameOfLifeBoard) 
{ 
    // Run the game of life until the board is empty or the exact same board is seen twice 
    // Important note: this method does NOT necessarily terminate!! 
    def go : Unit = 
    { 
    var currentBoard = initialBoard 
    var previousBoards = List[GameOfLifeBoard]() 

    while (!currentBoard.empty && !(previousBoards contains currentBoard)) 
    { 
     print(27.toChar + "[2J") // ANSI: clear screen 
     print(27.toChar + "[;H") // ANSI: move cursor to top left corner of screen 
     println(currentBoard.toString) 
     Thread.sleep(75) 

     // Warning: unbounded list concatenation can result in OutOfMemoryExceptions ####TODO: replace with LRU bounded list 
     previousBoards = List(currentBoard) ::: previousBoards 
     currentBoard = currentBoard tick 
    } 

    // Print the final board 
    print(27.toChar + "[2J") // ANSI: clear screen 
    print(27.toChar + "[;H") // ANSI: move cursor to top left corner of screen 
    println(currentBoard.toString) 
    } 
} 



// Script starts here 
val simple = Set((1,1)) 
val square = Set((4,4), (4,5), (5,4), (5,5)) 
val glider = Set((2,1), (3,2), (1,3), (2,3), (3,3)) 

val initialBoard = glider 

(new GameOfLife(new FiniteGameOfLifeBoard(20, 20, initialBoard))).go 
//(new GameOfLife(new InfiniteGameOfLifeBoard(initialBoard))).go 

// COPYRIGHT PETER MONKS 2010 

Một câu hỏi cụ thể: Tôi đã xóa các loại trả về từ tất cả các chức năng tại một điểm (Scala type inference ftw!), Nhưng mã thực sự khó đọc hơn. Có bất kỳ quy ước nào về thời điểm để lại các loại trả lại trong khi cho phép Scala tìm ra chúng (ngoài những trường hợp cần thiết)?

+1

Đặt mở cú đúp trong cùng một dòng. Nếu bạn viết mã trong Java, hãy làm như vậy ở đó. – OscarRyz

+1

Cảm ơn, nhưng tôi là một anh chàng kiểu ANSI-C kiểu cũ-skool, vì vậy tôi không thể thay đổi phong cách cú đúp/thụt đầu của mình sớm. ;-) – Peter

+1

Đó là những gì tôi nghĩ :) Nhưng, bạn nói: * phê bình mã của tôi * :) Tôi nghĩ rằng đó giống như học một ngôn ngữ tự nhiên, bạn sẽ luôn có một giọng nước ngoài. – OscarRyz

Trả lời

1

Tuple2 (và tất cả TupleN) loại giá trị có thể được viết với dấu ngoặc:

type cell = (Int, Int) 
// same as: 
type cell = Tuple2[Int, Int] 

val cell00 = (0, 0) 
// same as: 
val cell00 = Tuple2(0, 0) 
+0

Xin cảm ơn! Điều này có nghĩa là trong trường hợp "type cell = (Int, Int)", "cell" chỉ là cú pháp đường cho "Tuple2 [Int, Int]", hay chúng là các loại khác nhau? – Peter

+0

@Peter: Chúng là * không * loại khác nhau. Scala's 'type' giống Haskell ở chỗ nó chỉ định nghĩa các bí danh cho các kiểu hiện có, không phải là các bí danh mới. –

+0

Đẹp! Có một quy ước để sử dụng vốn ban đầu cho tên của các loại? I E. "Ô" được ưu tiên hơn "ô" – Peter

1

Vòng lặp for để tạo kết quả toString là ít hơn rất nhiều nhỏ gọn hơn so với mong muốn.

Bạn muốn

yRange.map(y => { 
    xRange.map(x => if (alive(x,y)) "#" else ".").mkString(" ") 
}).mkString("\n") 

Cũng a until b+1 được viết tốt hơn như a to b.

P.S. Điều (i => { là lý do chính tôi chuyển từ kiểu dấu ngoặc đơn riêng sang kiểu cuối dòng.

+0

Cảm ơn! Tôi đã tự hỏi liệu có một phiên bản chức năng hơn của toString không, nhưng đã không nhìn sâu vào nó. Và cảm ơn vì đã đề cập đến "để" quá - đã bỏ lỡ bằng cách nào đó. Về việc định vị các dấu ngoặc nhọn, trong Groovy (mà tôi cũng khá quen thuộc), tôi thấy mình đang đặt các dấu ngoặc nhọn cho các bao đóng trên một dòng mới - để đôi mắt của tôi dễ dàng quét nhanh một tệp lớn hơn cho "các khối mã" . Điều đó nói rằng nó kết quả trong một số trường hợp bất thường (như "})" trên một dòng) và có lẽ Scala cú pháp làm cho những trường hợp lẻ thậm chí còn phổ biến hơn. – Peter

2

Tuples thật tuyệt, nhưng mọi thứ có thể dễ đọc hơn nếu bạn đã tạo và sử dụng lớp "Ô".

+0

Cảm ơn! Cho rằng một ô không có gì khác hơn là một cặp Ints, bạn có gợi ý làm điều này như một lớp mới, hay là một "kiểu" như Randall gợi ý ở trên không? – Peter

+3

lớp mới: Trường hợp lớp tế bào (val x: Int, val y: Int) Nó không phải là một chiến thắng lớn, nhưng một lớp mới trong Scala là dễ dàng như vậy, và .x và .y trông rất đẹp hơn. Ném trong trường hợp phù hợp với mô hình lớp và phương pháp "copy" của 2.8, và nó có vẻ đáng giá. –

1

Tôi đã có một ý tưởng hay để thêm đồng thời, nhưng chưa bao giờ có được nó. Dù sao, here's một trong những triển khai của riêng tôi về nó. Tôi lên kế hoạch để viết blog về nó - và nhiều biến thể về chủ đề - nhưng cuộc sống đã cản trở. :-)

Đối với câu hỏi của bạn, tôi muốn luôn khai báo kiểu trả về của phương pháp của tôi, trừ khi nó thực sự là một lớp lót rất đơn giản.

+0

Tôi đoán nó phụ thuộc vào sở thích công cụ của bạn. Hầu hết các IDE đều hiển thị các kiểu trả về, không cần phải xem chúng hai lần. Đặt tên tốt cũng giúp: nếu bạn cần chú giải kiểu để tìm ra rằng một phương thức có tên 'toString' trả về một' Chuỗi', bạn có lẽ không nên lập trình :-) Tương tự với các phương thức có tên 'canSomething',' isSomething', ' hasSomething' cũng như 'equals' hoặc' contains' và kiểu trả về 'Boolean'. –

+1

@ Jörg Không chỉ về sở thích công cụ! Nó có thể làm bạn ngạc nhiên, nhưng có rất nhiều người nghĩ rằng phương pháp đặt tên là một cái nạng xung quanh các loại xấu, và rằng các loại của một chức năng nên là cơ sở chính để hiểu nó. Đây thường là những người tiếp tục yêu cầu Scala tương đương với http://www.haskell.org/hoogle/. :-) –

5

Bạn có thể thay

protected def neighbours(cell : Tuple2[Int, Int]) : Set[Tuple2[Int, Int]] = 
    Set((cell._1-1, cell._2-1), (cell._1, cell._2-1), (cell._1+1, cell._2-1), 
    (cell._1-1, cell._2),       (cell._1+1, cell._2), 
    (cell._1-1, cell._2+1), (cell._1, cell._2+1), (cell._1+1, cell._2+1)) 

với:

protected def neighbours(cell : Tuple2[Int, Int]) : Set[Tuple2[Int, Int]] = { 
    val direction = Set(-1,0,1) 
    for(x <- direction; y<-direction; if(x !=0 || y != 0)) 
    yield (cell._1+x,cell._2+y) 
} 

var previousBoards, var currentBoard: Nếu bạn xác định các phương pháp đi đệ quy bạn có thể xác định những như Vals.

while (!currentBoard.empty && !(previousBoards contains currentBoard)): Nếu bạn triển khai Iterable với InfiniteGameOfLifeBoard bạn có thể sử dụng biểu thức cho ở đây.

Điều hiển nhiên: Tại sao tài liệu ở định dạng không thể xử lý bằng scaladoc?

0

here's một cuộc sống rất ngắn Conway viết bằng một phong cách chức năng thuần túy

Các vấn đề liên quan