Revision cd3db92f-e64b-4947-9ca0-3e53bfe2b51d - Code Review Stack Exchange

Performance
===========

Compiler flags
--------------

Your main performance gain will simple be using the `-O3` flag for compilation. You may very well achieve even better performance with more sophisticated flags **depending on your compiler** and system. On my system it takes ~4s.

Output
------

Using a the simple linux profiling tool perf, shows that > 60 % of the remaining time is actually spent in `vfprintf`. A specific performance analysis tool recommendation **depends on your OS**.

Disabling the output, the remaining runtime is ~0.2s.

So you could now go ahead and try to optimize the output formatting, but is it really worth it for your use case?

Style
=====

Don't use `sprintf`
-------------------

`sprintf` is [very dangerous](http://stackoverflow.com/q/3662899/620382). Don't use it, not in C, nor in C++. In C++ concatenate your `std::string`s with `+` or use `std::stringstream`. If you really want format like syntax use [`boost::format`](http://www.boost.org/doc/libs/1_53_0/libs/format/doc/format.html).

Split up your main function
---------------------------

Don't handle file name computations and math stuff in the same function. It's very distracting.

Dont use `using namespace std;`
---------------------------

Yes, it requires more typing, but its bad practice. For an explanation take a look at the answers to [this question](http://stackoverflow.com/q/1452721/620382).

Use (const) references when appropriate
---------------------------------------

*Disclaimer*: I don't know Eigen, but I assume `MatrixXd` is an expensive-to-copy object.

You should use references to hand expensive-to-copy objects to functions. If they are input only, use const references - if they are in/out non-const references. So for `save_mat` use a `const MatrixXd&` just like you do for the file name string.

One could argue that `auto current_V = prev_V ... ` is also dangerously expensive because it requries assignment of expensive-to-copy objects. But in this case it is an rvalue so move assignment can be used. You could also use `const auto& current V = ...` instead which would express the intent of not modifying it later and perform slightly better (the latter being probably irrelevant)

*Note* The rest is mainly C++11 biased.

Loops
-----

In C++11, avoid raw loops. Use range-based loops instead.

Especially with the vector rcreated by arrange - It's neither efficient nor very clear to store the values you are going to process in the `t_vector` that you can easily generate on the fly instead of wasting memory on that. Usually I would recommend looking at [cppitertools](https://github.com/ryanhaining/cppitertools#range) or [range-v3](https://github.com/ericniebler/range-v3), which allows for a more efficient and cleaner abstraction of the t values.

Also use range-based loops for the outer loop through `dt_array`. You can also replace the innermost loop with with cppitertools/range-v3.

Use `const` or `constexpr`....
-----------------------------

... for local variables that don't change. Further define one variable per line and set it's value right at the definition (even if it is not const).

Use `std::array` instead of array[]
-----------------------------------

Simply use a `constexpr n_dt` and stay away for those `sizeof` hacks to figure out the number of elements you just defined a few lines earlier.