Progress

We have now finalized the state of the test suite and moved onto refactoring! Crucial to this was finding and fixing bugs, as well as writing a full testing suite, all detailed in previous blog posts. We are now onto the scary part, redesigning and refactoring the library. We have decided on a standard (C++ 14) and enforced it in our CMake config, and have also decided what portions of the library to compile as CXX. We can now begin to design our new C++ layer.

Being safe

On my suggestion we decided that now was a good break point at which to change target branches in the GitLab repo. Everything we had done so far was targeted at the the master repo, which was temporarily broken several times (uh oh). As refactoring will result in dramatic changes to the API, we decided to change to a new target branch C++-development which will allow dramatic changes in structure to be approved and played with without any chance of damaging the master branch.

Grand Designs

Speaking of refactoring, a prototype class setup for a refactored interface can now be seen in !28, which is serving as a focal point for our discussions. Our first aim is to get something that works up and running, which means I will likely start from something much simpler first, however !28 is leading to some good discussions as to how we might structure the ensuing code.

Things that have emerged include.

  • First we can wrap the existing structs in classes, before adding functionality, this will enable test driven development.
  • A focus on RAII, we should aim to build the objects as a whole and do lots of work in constructors wherever possible.
  • Use classes and subclasses, Blocks are a perfect example of this, as they share many common methods and traits, an example of this is given below.
  • This is also crucial for vectors that can accept subclassed <T> params
  • I think I will really have to follow K.I.S.S principles at first, or else I will wind up with something that is very “well designed” but nonfunctional.
  • File IO should be done by specialist classes ie TngFileReader and TngFileWriter

how to set up the right type of class polymorphism for a block is shown below, where different blocks all implement their own header length calculators by overriding the base class method.


class Block 
{
public:
    virtual CalculateHeaderLen();
};

class GeneralBlock : public Block
{
public:
    CalculateHeaderLen() override;
}

class TrajectoryFrameSet : public Block
{
public:
    CalculateHeaderLen() override;
}

Different blocks can then be held in the same vector (I think) as such:

std::vector<Block> BlockList;
BlockList.emplace_back(GeneralBlock);
BlockList.emplace_back(TrajectoryFrameSet);

Some things that I have been wondering?

  • Should the writing/reading work be assigned to each individual block subclass? That way they perfectly encapsulate their own data by calling a chain of writers or readers.
  • Can the TngTrajectory class be truly 2 way, ie can it be the sole class destination class for read information and the sole origin for information to be written?
  • Should we split the refactored interface into several source files? I am having a little bit of trouble with only having a single translational unit.

Zap

One of the things that prompted the move to a new branch is a very large change required for refactoring. Almost all of the functions in the current API in tng_io.c are declared static and as such cannot be used outside of that source file. This is a problem for writing a refactored interface, as I need access to these functions. This a lot of time this week has been spent stripping static from every single function and exposing them in the tng_io.h header along with their docstrings. The result of this can be seen in !29. Modification of the tng_io.c file also triggered clang-tidy to complain a lot, requiring modernization of null pointers:

// don't do this
int *p = 0;
// do this instead
int *p = nullptr;

something i didn’t realise is that clang-tidy can do inline fixes with –fix! Wow I wasted some time fixing pointers by hand until I found that. Crucial to getting the right fixes as well is compiling with -DCMAKE_EXPORT_COMPILE_COMMANDS=ON which can provide the right header info to clang-tidy if the resulting .json file is provided with -p. Something else puzzling was pipeline failures on clang-format despite no diff being shown by clang-format-diff locally. This was fixed by downgrading clang-format to 7.

More tests

Briefly, I have added some placeholder low level API tests for the C++ layer, in !31 mainly so I can get some test driven development going smoothly.

Next steps

Full steam ahead on refactoring! Paul and I are meeting tonight to talk design so that should be illuminating. !29, !28 and !30 need to be merged also, but have been a bit slow because of the cutoff for GROMACS 2020.3 is making people super busy.