Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More checks #10

Open
4 of 14 tasks
stil4m opened this issue Feb 22, 2017 · 9 comments
Open
4 of 14 tasks

More checks #10

stil4m opened this issue Feb 22, 2017 · 9 comments
Labels

Comments

@stil4m
Copy link
Owner

stil4m commented Feb 22, 2017

  • Exceeded line length
  • Unnecessary List.concat when concatenating only fixed size lists.
  • Record type aliases should be formatted multiline when exceeding N fields. (Record type aliases should be formatted multiline when exceeding N fields #24)
  • Functions where 'too much' happens.
  • Function should be moved to another module for better encapsulation.
  • Undocumented function that is exposed by module.
  • Use alias for complex 'thing'. This can be done by inspecting signatures.
  • Use point free notation where possible (\_ -> b to always b and \x -> List.map f x to List.map f)
  • Determine untested code.
  • Use multiline string instead of concatenated single line string.
  • Use list concatenation instead of multiple ++.
  • Replace function with a function provided by elm-lang/* or elm-community/*.
  • Magic numbers.
  • Check for unnecessary port modules (Add UnnecessaryPortModule check #45)
@eeue56
Copy link
Contributor

eeue56 commented Feb 25, 2017

@zwilias
Copy link
Contributor

zwilias commented Feb 26, 2017

  • use of Random iso Random.Pcg
  • using current time as a random seed rather than a port

@felixLam
Copy link
Contributor

felixLam commented Mar 6, 2017

  • For functions that have a record as an argument: check if only few (e.g. up to 3/configurable) fields are used and suggest to convert to individual parameters.

One of the "big code-smells" to me are view function that take in the whole model to print e.g. one field. This makes the view function very "potent".

@zwilias
Copy link
Contributor

zwilias commented Mar 16, 2017

Use point free notation where possible (\_ -> b) to always b and \x -> List.map f x to List.map f)

While I definitely agree with the second point, I do want to note that the first one seems in direct conflict with the noRedInk elm-style-guide which makes a decent case for \_ -> over always.

There's also the case to be made for eschewing overly point free code in performance critical code (say the inner loops of datastructure manipulation), though that's more relevant for library code as opposed to application code.

Sort of going on a tangent here, but I just realized that you'll be able to differentiate between application bundles and library code in 0.19. Might want to create slightly different presets for those..

@stil4m
Copy link
Owner Author

stil4m commented Mar 17, 2017

@zwilias Thanks for pointing that out. This kind of information is valuable :).

@ocharles
Copy link

#89 requests a warning on excessively indented code.

@gabrielperales
Copy link

Another simple metric to check is to count the number of lines in modules and functions. More than once I have found myself trying to understand code from others with files with thousands of lines and functions with hundreds of lines. I like the way how plato shows that info and the complexity of the code.

@adeschamps
Copy link
Contributor

Matthew Buscemi just published an Atom plugin, elm-lens, which (among other things) warns about items that are exposed but not externally referenced.

  • Exposed item with no external references

This might be a case where applications and libraries should be treated differently.

@akoppela
Copy link

akoppela commented Feb 28, 2018

  • Internal module used outside of it's related module.

E.g.

I have ModuleName and ModuleName.Internal. I want to be sure that ModuleName.Internal could be imported only by ModuleName. And not by some other modules.

Good

module ModuleName exposing (something)

import ModuleName.Internal

Bad

module OtherModuleName exposing (something)

import ModuleName.Internal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants