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

[EPIC] Design improvements #32

Open
2 of 11 tasks
jonblack opened this issue Jun 10, 2016 · 0 comments
Open
2 of 11 tasks

[EPIC] Design improvements #32

jonblack opened this issue Jun 10, 2016 · 0 comments

Comments

@jonblack
Copy link
Contributor

jonblack commented Jun 10, 2016

Looking at the code there are a lot of improvements that can be made to the design to make the code easier to understand and maintain. As new issues are discovered, add them here:

Let's chat before anything gets implemented.

  • Refactor functions that take too many parameters. Too many is something you'll know when you see it (e.g. msa::perform_msa_round_gapped)
  • Use smart pointers (std::shared_ptr and std::unique_ptr)
  • Improve comments (see Improve comments in code #31)
  • Don't pass by non-const reference. It's unclear in the caller that the value might be changed. Use a pointer instead.
  • Run cppcheck --enable=all src/ and fix issues. Read its documentation because unusedFunction is run and it can produce false positives.
  • Use OOP for methods that operate on an object (e.g. make_string only ever operates on a Sequence so should be a member function of Sequence).
  • Create Alignment class. This term appears very often. It's comparison operators should implement seq_data::compare_alignments so you can then do if alignment1 == alignment2 in the code.
  • Let the compiler infer the type using auto. This makes code much more readable (assuming your function name indicate clearly what's happening).
  • Give functions meaningful names (e.g. fasta::make_string should be fasta::sequence_to_string or better Sequence::to_string)
  • Rename all the fasta stuff to something else because it's not fasta and that's confusing.
  • Do we really need python scripts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant