Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Similar typedefs for parametric shape #1462

Merged
merged 5 commits into from Mar 21, 2020
Merged

Conversation

akrah
Copy link
Contributor

@akrah akrah commented Feb 27, 2020

PR Description

This PR:

  • homogenizes typdefs of parametric shapes that could be Points/Points2D and Vector/Vector2D
  • modify constructors/destructors by using delete/default directives
  • adds copy constructors that were strangely forbidden

This PR also:

  • fixes computation of bounding boxes for some shapes
  • removes some useless temporary variables
  • improves comparisons to 0 by using isAlmostEqual method
  • adds const directives to several method parameters

Checklist

  • Unit-test of your feature with Catch.
  • Doxygen documentation of the code completed (classes, methods, types, members...)
  • Documentation module page added or updated.
  • New entry in the ChangeLog.md added.
  • No warning raised in Debug cmake mode (otherwise, Travis C.I. will fail).
  • All continuous integration tests pass (Travis & appveyor)

- Use of delete/default directives for constructors/destructors
- Add copy constructors
@akrah
Copy link
Contributor Author

akrah commented Feb 28, 2020 via email

@copyme
Copy link
Member

copyme commented Feb 28, 2020

@akrah cool! I thought that these are mainly used for examples and demos, though. So, I was a bit surprised to see PR regarding them :-)

@akrah
Copy link
Contributor Author

akrah commented Feb 28, 2020

@copyme I suppose it was probably the case because PRs I make are mostly about difficulties to homogenize shapes and estimators concepts in order to use generalization in code using them.

@copyme
Copy link
Member

copyme commented Feb 28, 2020

@akrah I agree. During my masters Yukiko asked us to write some experiments in DGtal concerning estimators.
I wanted to have a very automatic and generic code and ended up with many adapters, etc. I remember having one big file with a description of an experiment, shapes, 2D/3D, estimator, etc. The program just read it and provided outputs with figures and values.

Unifying this is a marvelous idea, at least from a student stand point :D

- Simplify some methods computing parameter, derived, tangent, etc. by suppressing useless variables and defining constants
- Fix some comparisons to 0 with an isAlmostEqual method
- Add some const directives and change copied parameters by references
@dcoeurjo
Copy link
Member

Thx @akrah , could tou please add a Changelog entry ?

@akrah
Copy link
Contributor Author

akrah commented Mar 20, 2020

Done @dcoeurjo!

ChangeLog.md Outdated Show resolved Hide resolved
@dcoeurjo
Copy link
Member

thanks @akrah for the cleanup !
all good for me

@dcoeurjo dcoeurjo merged commit e78a139 into DGtal-team:master Mar 21, 2020
dcoeurjo pushed a commit that referenced this pull request Oct 2, 2020
dcoeurjo added a commit that referenced this pull request Oct 2, 2020
Similar typedefs for parametric shape
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants