[GenABEL-dev] [Genabel-commits] r1837 - in pkg/OmicABELnoMM: . examples src tests

[Frank, Alvaro Jesus]

The if statements below are a pain to my eyes :-). Due to the formatting
(missing indentation and/or missing curly braces) it gives the
impression that both the cout and the i = statement are carried out if
the condition is true (or maybe even that nothing happens in the
if-statement if you take the lack of indentation seriously).

At the very least I would indent the cout statement, but given that it
is a long line anyway (which are best avoided), I would add {}s and
break the cout statement over multiple lines.

I will do a code cleanup and address this issues.



> + if(params.mpi_id == 0)
> cout << "Warning, ID names between -g file and interactions file do not coincide! The files must have the same meaningful order!" << endl;
> i = params.n;//exit
> }
> @@ -433,7 +461,7 @@
> }
> }
> }
> - if(interaction_missings)

This if statement is indented as it should be, although I would prefer
breaking the long line.
Editor and screen size dependent, but I can comply.


Have you ever considered using C++ vectors instead of arrays? They save
you the trouble of new/delete, for example. I have no idea if there is a
performance hit, however. But I can imagine that combined with iterators
they may be well-optimised.
Or do you simply use them because LAPACK needs arrays anyway?

ISO C++ forbids variable-size array. I cannot comply with every style and avoid all possible warnings at the same time.
> Modified: pkg/OmicABELnoMM/src/main.cpp
> ===================================================================
> --- pkg/OmicABELnoMM/src/main.cpp 2014-10-17 14:57:34 UTC (rev 1836)
> +++ pkg/OmicABELnoMM/src/main.cpp 2014-10-21 13:30:17 UTC (rev 1837)
> @@ -18,39 +18,41 @@
> -d <0.0~1.0> -r <-10.0~1.0> -b -s <0.0~1.0> -e <-10.0~1.0> -i -f";
>
> string helpcmd_expl =


Note that you can use the information generated by autotools for version
info. If you include the config.h file that is generated you can do it
like we did in ProbABEL, for example in src/usage.cpp:

void print_version(void) {
cout << PACKAGE
<< " v. " << PACKAGE_VERSION
<< "\n(C) Yurii Aulchenko, Lennart C. Karssen, Maarten Kooyman, "
<< "Maksim Struchalin, The GenABEL team, EMC Rotterdam\n\n";
cout << "Using EIGEN version " << EIGEN_WORLD_VERSION
<< "." << EIGEN_MAJOR_VERSION << "." << EIGEN_MINOR_VERSION
<< " for matrix operations\n";
}

The PACKAGE and PACKAGE_VERSION variables comes from config.h, and are
generated by autotools. OmicABELnoMM's src/config.h contains them as well.


I will try it

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.r-forge.r-project.org/pipermail/genabel-devel/attachments/20141021/c17bc14d/attachment.html>