Was kann an meinem ersten Haskell-Programm verbessert werden?

Question

Hier ist mein erstes Haskell-Programm. Welche Teile würden Sie besser schreiben?

-- Multiplikationstabelle
-- Gibt n*n Multiplikationstabelle zur Basis b zurück

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

-- Gibt n*n Multiplikationstabelle zur Basis b zurück
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 " "

Answer 1

Sie verwenden nichts von import Text.Printf.

Stilistisch verwenden Sie mehr Klammern als nötig. Haskeller tendieren dazu, Code besser lesbar zu finden, wenn er von überflüssigem Zeug wie diesem bereinigt ist. Anstatt etwas wie h x = f (g x) zu schreiben, schreiben Sie lieber h = f . g.

Hier wird eigentlich kein Int benötigt; (Integral a) => a sollte ausreichen.

foldl (++) x xs == concat $ x : xs: Ich vertraue darauf, dass das integrierte concat besser funktioniert als Ihre Implementierung.
Außerdem sollten Sie foldr bevorzugen, wenn die Funktion in ihrem zweiten Argument träge ist, wie (++) - weil Haskell träge ist, verringert dies den Stapelspeicher (und funktioniert auch mit unendlichen Listen).
Außerdem sind unwords und unlines Abkürzungen für intercalate " " und concat . map (++ "\n") respectively, d.h. "mit Leerzeichen verbinden" und "mit Zeilenumbrüchen verbinden (plus abschließender Zeilenumbruch)"; Sie können ein paar Dinge durch diese ersetzen.

Wenn Sie keine großen Zahlen verwenden, ist w = length $ takeWhile (<= n) $ iterate (* b) 1 wahrscheinlich schneller. Oder, im Falle eines faulen Programmierers, lassen Sie w = length $ toBase b n.

concat ( (replicate ((w+1)* (n+2)) "-" ) == replicate ((w+1) * (n+2)) '-' - bin mir nicht sicher, wie das Ihnen entgangen ist, Sie haben es gerade ein paar Zeilen weiter oben richtig gemacht.

Sie tun dasselbe mit concat spaces auch. Aber wäre es nicht einfacher, tatsächlich den Text.Printf Import zu verwenden und printf "%*s " w digits zu schreiben?

Answer 2

Hier sind einige Vorschläge:

• Um die Tabellarität der Berechnung offensichtlicher zu machen, würde ich die Liste [0..n] an die Funktion line übergeben, anstatt n zu übergeben.

• Ich würde die Berechnung der horizontalen und vertikalen Achsen weiter aufteilen, sodass sie als Argumente an binopTable übergeben werden anstatt dort berechnet zu werden.

• Haskell ist higher-order, und fast keine der Berechnungen haben etwas mit Multiplikation zu tun. Ich würde daher den Namen von mulTable in binopTable ändern und die tatsächliche Multiplikation als Parameter übergeben.

• Zuletzt ist das Formatieren einzelner Zahlen wiederholend. Warum nicht \x -> format x b w als Parameter übergeben, um den Bedarf an b und w zu eliminieren?

Der Gesamteffekt der von mir vorgeschlagenen Änderungen ist, dass Sie eine allgemeine Higher-Order-Funktion zum Erstellen von Tabellen für binäre Operatoren erstellen. Ihr Typ wird in etwa so aussehen

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

und Sie landen mit einer viel wiederverwendbaren Funktion - zum Beispiel sollten boolesche Wahrheitstabellen ein Kinderspiel sein.

Higher-Order und wiederverwendbar ist der Haskell-Weg.

Answer 3

Norman Ramsey gab ausgezeichnete hochrangige (Design-)Vorschläge; Hier sind einige auf niedriger Ebene:

• Zuerst konsultieren Sie HLint. HLint ist ein freundliches Programm, das Ihnen rudimentäre Ratschläge gibt, wie Sie Ihren Haskell-Code verbessern können!
  • In Ihrem Fall gibt HLint 7 Vorschläge. (hauptsächlich über überflüssige Klammern)
  • Ändern Sie Ihren Code entsprechend den Vorschlägen von HLint, bis es mag, was Sie ihm geben.
• Mehr HLint-ähnliches Zeug:
  • concat (replicate i "-"). Warum nicht replicate i '-'?
• Konsultieren Sie Hoogle, wenn Sie Grund zu der Annahme haben, dass eine Funktion, die Sie benötigen, bereits in den Haskell-Bibliotheken verfügbar ist. Haskell kommt mit einer Menge nützlicher Funktionen, also wird Hoogle oft nützlich sein.
  • Müssen Sie Zeichenfolgen verketten? Suchen Sie nach [String] -> String und voilà, Sie finden concat. Jetzt ersetzen Sie all diese Falten.
  • Die vorherige Suche schlug auch unlines vor. Tatsächlich passt dies noch besser zu Ihren Bedürfnissen. Es ist Magie!
• Optional: Pause einlegen und in Ihrem Herzen Neil M für die Erstellung von Hoogle und HLint danken und anderen danken, die andere gute Sachen wie Haskell, Brücken, Tennisbälle und Hygieneartikel machen.
• Jetzt, für jede Funktion, die mehrere Argumente desselben Typs akzeptiert, machen Sie deutlich, was was bedeutet, indem Sie ihnen aussagekräftige Namen geben. Dies ist besser als Kommentare, aber Sie können immer noch beides verwenden.

Also

-- Gibt n*n-Multiplikationstabelle in Basis b zurück 
mulTable :: Int -> Int -> String
mulTable n b =

wird zu

mulTable :: Int -> Int -> String
mulTable größe basis =

• Um den zusätzlichen Zeichenüberhang des vorherigen Vorschlags zu mildern: Wenn eine Funktion nur einmal verwendet wird und nicht sehr nützlich ist, setzen Sie sie im Bereich ihres Aufrufers in seiner where-Klausel, wo sie die Variablen des Aufrufers verwenden könnte, und sparen Sie sich so die Notwendigkeit, alles an sie zu übergeben.

Also

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

wird zu

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

• Sie können sogar line in die where-Klausel von mulTable verschieben; meiner Meinung nach sollten Sie das.
  • Wenn Sie feststellen, dass eine in einer anderen where-Klausel verschachtelte where-Klausel störend ist, dann schlage ich vor, Ihre Einrückungsgewohnheiten zu ändern. Meine Empfehlung ist, eine konsistente Einrückung von immer 2 oder immer 4 Leerzeichen zu verwenden. Dann können Sie überall leicht sehen, wo das where in dem anderen where steht. ok

Hier ist, wie es aussieht (mit einigen anderen Stiländerungen):

import Data.List
import Data.Char

mulTable :: Int -> Int -> String
mulTable größe basis =
  unlines $
  [ vertikaleKöpfe
  , MinuszeichenZeile
  ] ++ map zeile [0 .. größe]
  where
    vertikaleKöpfe =
      concat
      $ replicate (zellenbreite + 2) ' '
      : map horizontaleKopfzeile [0 .. größe]
    horizontaleKopfzeile i = format i basis zellenbreite
    MinuszeichenZeile = replicate ((zellenbreite + 1) * (größe + 2)) '-'
    zellenbreite = length $ toBase basis (größe * größe)
    zeile y =
      concat
      $ format y basis zellenbreite
      : "|"
      : map element [0 .. größe]
      where
        element x = format (y * x) basis zellenbreite

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

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 =
  leerzeichen ++ ziffern ++ " "
  where 
    ziffern
      | v == 0    = "0"
      | otherwise = map toAlphaDigit (toBase b v)
    leerzeichen = replicate (w - length ziffern) ' '

Answer 4

0) füge eine Hauptfunktion hinzu :-) zumindest rudimentär

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 "Verwendung: nntable n base"

1) führe ghc oder runhaskell mit -Wall aus; führe hlint durch.

Obwohl hlint hier nichts Besonderes vorschlägt (nur einige überflüssige Klammern), sagt dir ghc, dass du Text.Printf hier tatsächlich nicht benötigst...

2) versuche es mit base = 1 oder base = 0 oder base = -1

Answer 5

Wenn Sie mehrzeilige Kommentare verwenden möchten, benutzen Sie:

{-  Ein mehrzeiliger
   Kommentar -}

Verwenden Sie außerdem niemals foldl, verwenden Sie stattdessen foldl', wenn Sie es mit großen Listen zu tun haben, die gefaltet werden müssen. Es ist speicher-effizienter.