INTO-CPS-Association / fmi_utilities

1 stars 0 forks source link

feedback #1

Open CThuleHansen opened 5 years ago

CThuleHansen commented 5 years ago

dfithan: my opinionated feedback, take it or leave it:

  1. never ever ever use unsafePerformIO
  2. use Text instead of String, it’s faster
  3. you get one wildcard import, and it’s your prelude. all other imports should be qualified or import specific functions.
  4. you have lots of magic numbers and magic strings instead of configuration parameters
  5. put your language pragmas in package.yaml - fire and forget - none of the language pragmas you specified are unsafe
  6. compile with -Wall at least. there are some other warnings you can compile with but i forget where the complete list is
  7. you are redoing a lot of work around logging and web serving. your logging library and scotty do a lot of this work for you. for logging there’s a request logger middleware (http://hackage.haskell.org/package/wai-extra-3.0.28/docs/Network-Wai-Middleware-RequestLogger.html) for scotty see parseParam. things you did well:
  8. using Either and returning an intelligible result instead of failing indiscriminately
  9. likewise, decoding with Maybe for your configuration (edited)

TheMatten: ghc-options:

chriz-keera: https://downloads.haskell.org/~ghc/latest/docs/html/users_guide/using-warnings.html

hololeap (irc): you should probably be handling errors with Control.Exception.bracket as well. also, consider learning about conduit if you plan on doing anything like processing the file on the fly