search

mohitanand001 / underscore_cpp

underscore_cpp
MIT License
7 stars 30 forks source link

const iteration #11

Open mohitanand001 opened 6 years ago

mohitanand001 commented 6 years ago

@Gotham13121997 There are some functions , which take iterator as param. I think const_iterator would be better in cases where read only operations are needed. For pointers use https://stackoverflow.com/questions/5346890/what-is-the-difference-between-const-iterator-and-iterator

mohitanand001 commented 5 years ago

Changes needed here: https://github.com/farziengineer/underscore_cpp/blob/master/underscore.cpp#L60 https://github.com/farziengineer/underscore_cpp/blob/master/underscore.cpp#L76 https://github.com/farziengineer/underscore_cpp/blob/master/underscore.cpp#L91 https://github.com/farziengineer/underscore_cpp/blob/master/underscore.cpp#L104 https://github.com/farziengineer/underscore_cpp/blob/master/underscore.cpp#L152 https://github.com/farziengineer/underscore_cpp/blob/master/underscore.cpp#L167 https://github.com/farziengineer/underscore_cpp/blob/master/underscore.cpp#L182

gotham13 commented 5 years ago

I dont think its possible.

mohitanand001 commented 5 years ago

Okay I'll look into it this weekend.

gotham13 commented 5 years ago

cool

mohitanand001 commented 5 years ago

I tried it for sometime (an hour or so), but failed. Now we have two options.

  1. Convert functions signature like we have in min and max.
  2. Leave the functions as it is, as it allows for more flexibility, since we can iterate from any position of an stl to any other position (while in case of min and max functions the iterations are fixed from beginning to end). Option 2 sounds better, though it does not conform to "const-correctness". Anyway I will leave this issue open.
sszperling commented 5 years ago

Hi! I looked into this for a little while and I think the code is (mostly) ok as far as "const-correctness" goes. I might be wrong, but since the Iterator types are templated, the code should work with const containers and const_iterators as well as their non-const counterparts.

In some cases this might actually be desirable. For instance, you might want to modify an element found with find_if. However, if the container or the iterators are already const, the compiler will enforce that the predicate's operator() does not receive a non-const reference. In that regard, max and min actually restrict you significantly by only returning const_iterators.

There are some fringe cases though: functions that no not return an Iterator (such as every or any) should usually be fine with const_iterators. Also, ideally a Predicate never modifies a reference passed to it (always takes const refs or values). However, enforcing these concepts is nigh impossible in C++11 (or maybe even C++17, since the Concepts TS didn't make the cut).

Personally, I think the code is fine as-is in terms of "const-correctness", although the signatures are a bit inconsistent (max takes a Collection but find_if takes 2 Iterators). I wouldn't worry too much about it, it sounds like a user issue more than a library issue.