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

random access iterators in ArithmeticalDSL #801

Merged

Conversation

troussil
Copy link
Member

we provide random access services to iterators of ArithmeticalDSL/DSS

* @return distance between the two iterators
*/
Position distance_to(const ConstIterator& aOther) const;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In classical STL concepts, the return type of "distance_to" is an unsigned scalar.
Position could also have been typdefed as an unsigned version of TCoordinate, what do you think ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not agree. In STL, distances between random access iterators are
signed (they supports arithmetic operators + and -). For instance, for
pointers, difference_type is basically ptrdiff_t, which is a signed integer
type. See http://www.cplusplus.com/reference/iterator/iterator_traits/

2014-04-19 10:16 GMT+02:00 David Coeurjolly [email protected]:

In src/DGtal/geometry/curves/ArithmeticalDSL.h:

  •  /**
    
  •   \* Moves @a myCurrentPoint lying at position i to the
    
  •   \* point of the DSL lying at position i + @a aShift
    
  •   \* @param aShift position difference
    
  •   \* NB: in O(1)
    
  •   */
    
  •  void advance(const Position& aShift);
    
  •  /**
    
  •   \* Computes the distance between *this and @a aOther, ie.
    
  •   \* the difference between their positions
    
  •   \* @param aOther any other iterator
    
  •   \* @return distance between the two iterators
    
  •   */
    
  •  Position distance_to(const ConstIterator& aOther) const;
    

In classical STL concepts, the return type of "distance_to" is an unsigned
scalar.
Position could also have been typdefed as an unsigned version of
TCoordinate, what do you think ?


Reply to this email directly or view it on GitHubhttps://pull/801/files#r11792918
.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. My mistake I thought it was a size_t sort of thing..

Le 19 avr. 2014 à 18:44, troussil [email protected] a écrit :

In src/DGtal/geometry/curves/ArithmeticalDSL.h:

  •  /**
    
  •   \* Moves @a myCurrentPoint lying at position i to the 
    
  •   \* point of the DSL lying at position i + @a aShift
    
  •   \* @param aShift position difference
    
  •   \* NB: in O(1)
    
  •   */
    
  •  void advance(const Position& aShift);
    
  •  /**
    
  •   \* Computes the distance between *this and @a aOther, ie.
    
  •   \* the difference between their positions
    
  •   \* @param aOther any other iterator 
    
  •   \* @return distance between the two iterators
    
  •   */
    
  •  Position distance_to(const ConstIterator& aOther) const; 
    
    I am not agree. In STL, distances between random access iterators are signed (they supports arithmetic operators + and -). For instance, for pointers, difference_type is basically ptrdiff_t, which is a signed integer type. See http://www.cplusplus.com/reference/iterator/iterator_traits/ 2014-04-19 10:16 GMT+02:00 David Coeurjolly [email protected]:


    Reply to this email directly or view it on GitHub.

@dcoeurjo
Copy link
Member

otherwise this PR is fine... thanks.
(waiting for your comment on the typedef before merging)

@dcoeurjo
Copy link
Member

Okay, merging.. Could you please add a line to the changelog ?

@dcoeurjo
Copy link
Member

(Just push to the same branch)

dcoeurjo added a commit that referenced this pull request Apr 21, 2014
…ator

random access iterators in ArithmeticalDSL
@dcoeurjo dcoeurjo merged commit 9021574 into DGtal-team:master Apr 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants