libcellml icon indicating copy to clipboard operation
libcellml copied to clipboard

Code maintenance

Open agarny opened this issue 6 years ago • 0 comments

This issue aims at cleaning up the code, refactoring it whenever needed, etc. Things to be done (feel free to add to the list!):

  • [ ] As much as possible (and definitely in our comments), we should have 120 characters per line. (Yes, 80 characters used to be good when people didn't have wide screens.)
  • [ ] Go through the Clang-Tidy checks that have been disabled, and reenable whatever we can (e.g. modernize-use-trailing-return-type) (see issue #397).
  • [ ] Fix up implementation header file includes to follow the coding standard.
  • [ ] Make our lists of items consistent (see comment https://github.com/cellml/libcellml/pull/400#pullrequestreview-296551890), i.e. replace things like
const std::vector<std::string> expectedErrors = {
    "Encapsulation in model 'model_name' has an invalid child element 'component_free'."};

with

const std::vector<std::string> expectedErrors = {
    "Encapsulation in model 'model_name' has an invalid child element 'component_free'.",
};
  • [ ] Replace occurrences of "mathml" with "MathML".
  • [ ] Ensure that we use the same convention for file/folder names (e.g. folder_name, my_super_duper_file.txt).
  • [ ] Replace std::shared_ptr<Xxx> with XxxPtr wherever possible (e.g. Component::create()).
  • [ ] Replace calls to std::string::at() with std::string::operator[] if we know the index is valid and if, therefore, we don't handle a possible exception raised by std::string::at().
  • [ ] Replace std::list with std::vector wherever possible (std::vector is much faster than std::list).
  • [ ] Clean up while loops and conditionals (see discussion here).
  • [ ] Use XxxPtr rather than std::shared_ptr<const Xxx> (e.g. getVariableIndexInComponent()).
  • [ ] Ensure that documentation is consistent. That's a big one, but there is no consistency in our documentation, e.g. sometimes we use "Return" and other times "Returns" when describing the value returned by a function (see here).
  • [ ] Ensure that we don't create trivial variables for returning.
  • [ ] Ensure that we use British spelling (as opposed to US spelling).
  • [ ] Use the auto keyword whenever possible in variable declarations.
  • [ ] Replace bits of code like:
Importer::Importer()
    : mPimpl(new ImporterImpl())
{
    mPimpl->mImporter = this;
}

with:

Importer::Importer()
    : mPimpl(new ImporterImpl(this))
{
}
  • [ ] API documentation: use @ref rather than @c when wanting to reference a class in the API.
  • [ ] Some methods (e.g. doIsResolved() in src/api/libcellml/component.h) are included in the private section of of some of our classes. The private section of such classes (which are part of our API) should only have our constructors and our PIMP-related code. Nothing else.

agarny avatar Sep 26 '19 03:09 agarny