2009-12-28 18 views
5

Đây là chương trình Haskell đầu tiên của tôi. Những phần nào bạn sẽ viết một cách tốt hơn?Điều gì có thể được cải thiện trong chương trình haskell đầu tiên của tôi?

-- Multiplication table 
-- Returns n*n multiplication table in base b 

import Text.Printf 
import Data.List 
import Data.Char 

-- Returns n*n multiplication table in base b 
mulTable :: Int -> Int -> String 
mulTable n b = foldl (++) (verticalHeader n b w) (map (line n b w) [0..n]) 
       where 
       lo = 2* (logBase (fromIntegral b) (fromIntegral n)) 
       w = 1+fromInteger (floor lo) 

verticalHeader :: Int -> Int -> Int -> String 
verticalHeader n b w = (foldl (++) tableHeader columnHeaders) 
         ++ "\n" 
         ++ minusSignLine 
         ++ "\n" 
        where 
        tableHeader = replicate (w+2) ' ' 
        columnHeaders = map (horizontalHeader b w) [0..n] 
        minusSignLine = concat (replicate ((w+1)* (n+2)) "-") 

horizontalHeader :: Int -> Int -> Int -> String 
horizontalHeader b w i = format i b w 

line :: Int -> Int -> Int -> Int -> String 
line n b w y = (foldl (++) ((format y b w) ++ "|") 
          (map (element b w y) [0..n])) ++ "\n" 

element :: Int -> Int -> Int -> Int -> String 
element b w y x = format (y * x) b w 

toBase :: Int -> Int -> [Int] 
toBase b v = toBase' [] v where 
    toBase' a 0 = a 
    toBase' a v = toBase' (r:a) q where (q,r) = v `divMod` b 

toAlphaDigits :: [Int] -> String 
toAlphaDigits = map convert where 
    convert n | n < 10 = chr (n + ord '0') 
      | otherwise = chr (n + ord 'a' - 10) 

format :: Int -> Int -> Int -> String 
format v b w = concat spaces ++ digits ++ " " 
       where 
        digits = if v == 0 
          then "0" 
          else toAlphaDigits (toBase b v) 
        l = length digits 
        spaceCount = if (l > w) then 0 else (w-l) 
        spaces = replicate spaceCount " " 
+1

Bạn có kiểm tra? Họ có thể tiết lộ một số cải tiến của riêng họ. –

Trả lời

11

Dưới đây là một số gợi ý:

  • Để thực hiện tabularity của việc tính toán rõ ràng hơn, tôi sẽ vượt qua danh sách [0..n] đến line chức năng chứ không phải là đi qua n.

  • Tôi sẽ tiếp tục chia nhỏ tính toán của trục ngang và trục dọc sao cho chúng được chuyển làm đối số cho mulTable thay vì được tính ở đó.

  • Haskell là thứ tự cao hơn và hầu như không có tính toán nào liên quan đến phép nhân. Vì vậy, tôi sẽ thay đổi tên của mulTable thành binopTable và vượt qua phép nhân thực tế dưới dạng tham số.

  • Cuối cùng, định dạng của các số riêng lẻ là lặp đi lặp lại. Tại sao không vượt qua \x -> format x b w làm thông số, hãy loại bỏ nhu cầu cho bw?

Hiệu ứng ròng của những thay đổi mà tôi đề xuất là bạn tạo hàm bậc cao chung để tạo bảng cho toán tử nhị phân. loại của nó trở nên một cái gì đó giống như

binopTable :: (i -> String) -> (i -> i -> i) -> [i] -> [i] -> String 

và bạn gió lên với một nhiều thể tái sử dụng nhiều chức năng — ví dụ, bảng sự thật Boolean phải là một miếng bánh.

Thứ tự cao hơn và có thể sử dụng lại là cách Haskell.

+0

Cảm ơn bạn đã bình luận. – danatel

4

0) thêm một chức năng :-) chính ít nhất thô sơ

import System.Environment (getArgs) 
import Control.Monad (liftM) 

main :: IO() 
main = do 
    args <- liftM (map read) $ getArgs 
    case args of 
    (n:b:_) -> putStrLn $ mulTable n b 
    _  -> putStrLn "usage: nntable n base" 

1) chạy ghc hoặc runhaskell với -Wall; chạy qua hlint.

Trong khi hlint không cho thấy bất cứ điều gì đặc biệt ở đây (chỉ có một số dấu ngoặc không cần thiết), ghc sẽ cho bạn biết rằng bạn không thực sự cần Text.Printf đây ...

2) cố gắng chạy nó với cơ sở = 1 hay base = 0 hoặc base = -1

+1

Chắc chắn tôi không phải là người duy nhất chủ yếu chạy mã thông qua ghci, và chỉ thêm 'main' như một ý tưởng ... –

+0

' ghci' thật tuyệt, nhưng tôi chỉ chạy những đoạn nhỏ ở đó ... (vâng, ' : load' cũng hữu ích). – sastanin

+1

Tôi chạy các đoạn nhỏ của mã của tôi thông qua GHCi để kiểm tra nó, nhưng tôi luôn luôn có một tuyên bố chính trong giai đoạn cuối để tôi có thể chạy toàn bộ chương trình với runghc và kiểm tra nó. – Rayne

11

Bạn không sử dụng bất kỳ thứ gì từ import Text.Printf.

Theo phong cách, bạn sử dụng nhiều dấu ngoặc đơn hơn mức cần thiết. Haskellers có xu hướng tìm mã dễ đọc hơn khi nó được làm sạch những thứ không liên quan như thế. Thay vì một cái gì đó như h x = f (g x), viết h = f . g.

Không có gì ở đây thực sự yêu cầu Int; (Integral a) => a nên làm.

foldl (++) x xs == concat $ x : xs: Tôi tin tưởng được xây dựng trong concat để hoạt động tốt hơn việc triển khai của bạn.
Ngoài ra, bạn nên thích foldr khi hàm bị lười trong đối số thứ hai, vì (++) là – vì Haskell lười, điều này làm giảm không gian ngăn xếp (và cũng hoạt động trên danh sách vô hạn).
Ngoài ra, unwordsunlines là các phím tắt cho intercalate " "concat . map (++ "\n") tương ứng, tức là "kết hợp với dấu cách" và "kết hợp với dòng mới (cộng với dấu dòng mới)"; bạn có thể thay thế một vài thứ bằng những thứ đó.

Trừ khi bạn sử dụng số lớn, w = length $ takeWhile (<= n) $ iterate (* b) 1 có thể nhanh hơn. Hoặc, trong trường hợp lập trình viên lười biếng, hãy để w = length $ toBase b n.

concat ((replicate ((w+1)* (n+2)) "-") == replicate ((w+1) * (n+2)) '-' – không chắc chắn bạn đã bỏ lỡ điều này như thế nào, bạn đã nhận được ngay một vài dòng.

Bạn cũng làm như vậy với concat spaces. Tuy nhiên, sẽ không dễ dàng hơn khi sử dụng nhập khẩu Text.Printf và viết printf "%*s " w digits?

+0

Cảm ơn bạn đã bình luận. – danatel

2

Nếu bạn muốn có nhiều dòng bình luận sử dụng:

{- A multiline 
    comment -} 

Ngoài ra, không bao giờ sử dụng foldl, sử dụng foldl' thay vào đó, trong trường hợp bạn đang đối phó với các danh sách lớn mà phải gấp. Đó là bộ nhớ hiệu quả hơn.

+5

'foldl'' nếu chức năng là nghiêm ngặt,' foldr' nếu nó là lười biếng. –

1

Nhận xét ngắn gọn cho biết chức năng của từng chức năng, đối số và giá trị trả về của nó luôn tốt. Tôi đã phải đọc mã khá cẩn thận để hoàn toàn hiểu được nó.

Một số sẽ nói nếu bạn làm như vậy, có thể không yêu cầu chữ ký rõ ràng. Đó là một câu hỏi thẩm mỹ, tôi không có một ý kiến ​​mạnh mẽ về nó.

Một cảnh báo nhỏ: nếu bạn loại bỏ chữ ký loại, bạn sẽ tự động nhận được sự đa năng hỗ trợ đa hình được đề cập, nhưng bạn vẫn cần một khoảng toAlphaDigits vì hạn chế "monomorphism".

+0

Tôi có ý kiến ​​rằng, sau khi đặt tên chức năng tốt và chữ ký rõ ràng, các bình luận có thể không cần thiết. Và tôi muốn thể hiện bản thân mình trong Haskell giả mã (mà sẽ xảy ra để xem xét chính xác như mã Haskell) hơn trong tiếng Anh.Nhưng, như bạn lưu ý, đó là tùy thuộc vào hương vị thẩm mỹ của tác giả. – ephemient

+0

Đó là sự thật, tôi đoán nó chỉ đặt trọng tâm hơn vào tên, đặc biệt là tên tham số. Ví dụ, 'verticalHeader n b w' không thực sự nhảy ra ngoài và hét lên những gì người ta nên mong đợi hàm đó làm. – Dan

5

Norman Ramsey đưa ra các đề xuất cấp cao (thiết kế) xuất sắc; Dưới đây là một số cấp độ thấp:

  • Đầu tiên, tham khảo ý kiến ​​với HLint. HLint là một chương trình thân thiện cung cấp cho bạn lời khuyên thô sơ về cách cải thiện mã Haskell của bạn!
    • Trong trường hợp của bạn, HLint đưa ra 7 đề xuất. (chủ yếu là về dấu ngoặc thừa)
    • Sửa đổi mã của bạn theo đề xuất của HLint cho đến khi nó thích nội dung bạn cung cấp.
  • More thứ HLint như:
    • concat (replicate i "-"). Tại sao không phải là replicate i '-'?
  • Tham khảo ý kiến ​​Hoogle bất cứ khi nào có lý do để tin rằng chức năng bạn cần đã có sẵn trong thư viện của Haskell. Haskell có rất nhiều chức năng hữu ích để Hoogle có thể sử dụng khá thường xuyên.
    • Cần ghép nối các chuỗi? Tìm kiếm [String] -> String và voila bạn tìm thấy concat. Bây giờ hãy thay thế tất cả các nếp gấp đó.
    • Tìm kiếm trước đó cũng đề xuất unlines. Trên thực tế, điều này thậm chí còn phù hợp hơn với nhu cầu của bạn. Đó là phép thuật!
  • Tùy chọn: tạm dừng và cảm ơn trong tim của bạn về Neil M khi tạo Hoogle và HLint, và cảm ơn người khác vì đã tạo ra những thứ tốt khác như Haskell, cầu, quả bóng tennis và vệ sinh.
  • Bây giờ, đối với mọi chức năng nhận một số đối số cùng loại, hãy làm rõ nó có nghĩa là gì, bằng cách đặt tên mô tả cho chúng. Điều này tốt hơn bình luận, nhưng bạn vẫn có thể sử dụng cả hai.

Vì vậy

-- Returns n*n multiplication table in base b 
mulTable :: Int -> Int -> String 
mulTable n b = 

trở thành

mulTable :: Int -> Int -> String 
mulTable size base = 
  • Để làm mềm các nhân vật thêm đòn của đề nghị trước đó: Khi một chức năng chỉ được sử dụng một lần, và không phải là rất hữu ích bởi chính nó , đặt nó bên trong phạm vi của người gọi trong mệnh đề where của nó, nơi nó có thể sử dụng các biến của người gọi, giúp bạn tiết kiệm được mọi thứ cần thiết.

Vì vậy

line :: Int -> Int -> Int -> Int -> String 
line n b w y = 
    concat 
    $ format y b w 
    : "|" 
    : map (element b w y) [0 .. n] 

element :: Int -> Int -> Int -> Int -> String 
element b w y x = format (y * x) b w 

trở thành

line :: Int -> Int -> Int -> Int -> String 
line n b w y = 
    concat 
    $ format y b w 
    : "|" 
    : map element [0 .. n] 
    where 
    element x = format (y * x) b w 
  • Bạn thậm chí có thể di chuyển line vào where khoản mulTable 's; Imho, bạn nên.
    • Nếu bạn tìm thấy mệnh đề where được lồng trong mệnh đề where khác, sau đó tôi đề nghị thay đổi thói quen thụt đầu dòng của bạn. Khuyến nghị của tôi là sử dụng sự thụt đầu dòng nhất quán của luôn luôn 2 hoặc luôn luôn 4 không gian. Sau đó, bạn có thể dễ dàng nhìn thấy, ở mọi nơi, trong đó wherewhere khác là tại. ok

Dưới đây là những gì nó trông giống như (với một vài thay đổi khác trong phong cách):

import Data.List 
import Data.Char 

mulTable :: Int -> Int -> String 
mulTable size base = 
    unlines $ 
    [ vertHeaders 
    , minusSignsLine 
    ] ++ map line [0 .. size] 
    where 
    vertHeaders = 
     concat 
     $ replicate (cellWidth + 2) ' ' 
     : map horizontalHeader [0 .. size] 
    horizontalHeader i = format i base cellWidth 
    minusSignsLine = replicate ((cellWidth + 1) * (size + 2)) '-' 
    cellWidth = length $ toBase base (size * size) 
    line y = 
     concat 
     $ format y base cellWidth 
     : "|" 
     : map element [0 .. size] 
     where 
     element x = format (y * x) base cellWidth 

toBase :: Integral i => i -> i -> [i] 
toBase base 
    = reverse 
    . map (`mod` base) 
    . takeWhile (> 0) 
    . iterate (`div` base) 

toAlphaDigit :: Int -> Char 
toAlphaDigit n 
    | n < 10 = chr (n + ord '0') 
    | otherwise = chr (n + ord 'a' - 10) 

format :: Int -> Int -> Int -> String 
format v b w = 
    spaces ++ digits ++ " " 
    where 
    digits 
     | v == 0 = "0" 
     | otherwise = map toAlphaDigit (toBase b v) 
    spaces = replicate (w - length digits) ' ' 
+1

Cảm ơn bạn rất nhiều. Với câu hỏi này tôi thực sự không vui vì tôi không thể chấp nhận nhiều hơn một câu trả lời! Tôi đã học được rất nhiều từ phản hồi của bạn. – danatel

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