ethz-asl / curves

A library of curves for estimation.
BSD 3-Clause "New" or "Revised" License
74 stars 28 forks source link

Feature/linear interpolation #12

Closed gawela closed 10 years ago

gawela commented 10 years ago

We implemented a first version of linear interpolations. It still needs some polishing, but you can have a look already. Gtests will follow.

furgalep commented 10 years ago

One general point: you are not following the formatting guidelines. Please add automatic formatting to your IDE:

https://github.com/ethz-asl/programming_guidelines/wiki/Code-formatter

Common things I'm seeing:

No space between condition and brace:

do

for(int i = 0; i < 10; ++i) {
  // ...
}

not

for(int i = 0; i < 10; ++i){
  // ...
}

and not

for(int i = 0; i < 10; ++i)
{
  // ...
}

Incorrect indentation

Indentation should be 2 spaces...not a tab, not four spaces.

Please fix all of these and review the style guide for others.

gawela commented 10 years ago

We have incorporated the changes you suggested. Please have anther look. Please also look at comments...

furgalep commented 10 years ago

Hi @gawela and @rdube. I'm sure you see this

"We can’t automatically merge this pull request. Use the command line to resolve conflicts before continuing."

This is because Sean's edits will have conflicts with yours. It won't be difficult to resolve. Do you need short instructions how to do this?

rdube commented 10 years ago

We have incorporated the second batch of comments. This one from @furgalep about HermiteCoefficientManager::getCoefficientsAt still remains:

Use pointers for output arguments.

Would you mean declaring the function as following instead?

bool getCoefficientsAt(Time time, std::pair *outCoefficients) const;

And calling it as :

std::pair< KeyCoefficientTime, KeyCoefficientTime > rval; manager_.getCoefficientsAt(time, &rval);

If yes, should we apply this passing by pointer principle to all functions which passed by reference?

furgalep commented 10 years ago

We are following the Google style that output arguments should be passed by pointer. Input arguments can still be passed by const reference.

rdube commented 10 years ago

OK we have merged manually to include Sean's modifications and it builds on out side.

furgalep commented 10 years ago

This still can't be merged with the master. Please fix this and commit the changes so that I can merge. Great work!