search

DGtal-team / DGtal

Digital Geometry Tools and Algorithm Library
https://dgtal.org
GNU Lesser General Public License v3.0
370 stars 115 forks source link

KSpace stored as reference in GridCurve and iterators #233

Closed JacquesOlivierLachaud closed 11 years ago

JacquesOlivierLachaud commented 13 years ago

It seems KSpace are stored by value in GridCurve and a few iterators. I suggest to store them as const KSpace & mySpace instead. This is because the next generation of KhalimskySpace will have a lot more data (this is to speed up cell coding). Another possibility is a CountedPtr or a CowPtr, but the space is still duplicated at least once (when giving a KSpace as parameter).

troussil commented 13 years ago

I agree with you. It has to be corrected in GridCurve.h and Modifier.h.

From: Jacques-Olivier Lachaud reply@reply.github.com Sent: Sat Oct 01 22:10:51 CEST 2011 To: troussil tristan.roussillon@liris.cnrs.fr Subject: [DGtal] KSpace stored as reference in GridCurve and iterators (#233)

It seems KSpace are stored by value in GridCurve and a few iterators. I suggest to store them as const KSpace & mySpace instead. This is because the next generation of KhalimskySpace will have a lot more data (this is to speed up cell coding). Another possibility is a CountedPtr or a CowPtr, but the space is still duplicated at least once (when giving a KSpace as parameter).

Reply to this email directly or view it on GitHub: https://github.com/DGtal-team/DGtal/issues/233

troussil commented 12 years ago

changed (stored as an aliasing pointer) in the pending pull request: https://github.com/DGtal-team/DGtal/pull/267

JacquesOlivierLachaud commented 12 years ago

Ok for the ranges, but it is not the case for GridCurve yet.

troussil commented 12 years ago

You 're right. I will correct this before the end of the week.

By the way, I also wanted to only store one kind of scells (of dimension 1 or d-1) instead of two (of dimension 0 and 1 respectively)

2012/4/16 Jacques-Olivier Lachaud < reply@reply.github.com

Ok for the ranges, but it is not the case for GridCurve yet.

  • GridCurve stores an object KSpace
  • this requires CCellularGridSpaceND to be DefaultConstructible and CopyConstructible.
  • I suggest storing a const pointer to a KSpace which implies:
    • you can keep a default constructor for GridCurve
    • you only store a pointer to the space
    • you can test if the GridCurve is valid
    • you can add a method to init the GridCurve for another KSpace.

Reply to this email directly or view it on GitHub: https://github.com/DGtal-team/DGtal/issues/233#issuecomment-5149835

JacquesOlivierLachaud commented 12 years ago

Ok. I think it is reasonable since 0 or n-2 cells can be obtained from those cells.

troussil commented 12 years ago

In CCellularGridSpaceND.h models of CCellularGridSpaceND are copy constructible but not default constructible. Is it correct ?