Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-10-04 Thread Chris Olivier
-0.5 if keeping it as a warning. On Thu, Oct 4, 2018 at 1:23 AM kellen sunderland < kellen.sunderl...@gmail.com> wrote: > I agree that active C++ developers should be the ones making these > choices. The downside to that is that many of these people are already > pretty busy. To make the best

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-10-04 Thread kellen sunderland
I agree that active C++ developers should be the ones making these choices. The downside to that is that many of these people are already pretty busy. To make the best use possible of their time it would probably make sense to create a concise doc with proposed style changes and ETAs rather than

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-10-03 Thread Pedro Larroy
+1 @Chris: do you have data on the performance difference? As far as I know there's a "rewrite rule" like the one between lambdas and C++ functors, so performance should be very well defined, but maybe there's something that you are point out that we are missing. Having a consistent and concise c

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-30 Thread Chris Olivier
unless you don’t think that’s reasonable... On Sun, Sep 30, 2018 at 7:59 AM Chris Olivier wrote: > If you get three +1’s from the top 6 contributors of C++ code (by volume), > I’ll switch to -0, since the ones committing the most C++ code will be the > most impacted and probably it should be the

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-30 Thread Chris Olivier
If you get three +1’s from the top 6 contributors of C++ code (by volume), I’ll switch to -0, since the ones committing the most C++ code will be the most impacted and probably it should be their decision, imho. On Sun, Sep 30, 2018 at 12:28 AM kellen sunderland < kellen.sunderl...@gmail.com> wrot

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-30 Thread kellen sunderland
About 60 but they're all addressed In the ref PR. On Sun, Sep 30, 2018, 6:12 AM Chris Olivier wrote: > How many errors exist in the code base right now if it were to be enabled? > > On Sat, Sep 29, 2018 at 7:03 PM Naveen Swamy wrote: > > > Thanks Kellen & Anton, for your detailed explanation an

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-29 Thread Chris Olivier
How many errors exist in the code base right now if it were to be enabled? On Sat, Sep 29, 2018 at 7:03 PM Naveen Swamy wrote: > Thanks Kellen & Anton, for your detailed explanation and links to > advantages, appreciate it. > changing my vote to *-0*, I suggest to show as warnings. > > On Sat, S

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-29 Thread Naveen Swamy
Thanks Kellen & Anton, for your detailed explanation and links to advantages, appreciate it. changing my vote to *-0*, I suggest to show as warnings. On Sat, Sep 29, 2018 at 8:06 PM Anton Chernov wrote: > And if you want a more authoritative opinion on that check out what the C++ > core guidelin

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-29 Thread Anton Chernov
And if you want a more authoritative opinion on that check out what the C++ core guidelines are saying [1]: > ES.71: Prefer a range-for-statement to a for-statement when there is a choice > Reason > Readability. Error prevention. Efficiency. Best regards Anton [1] https://github.com/isocpp/CppCo

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-29 Thread Anton Chernov
+1 Maybe it's not necessary to enforce usage of range-based for, but I would highly encourage to to it due to already named advantages. If code would be introduced using the old-style there could be a comment suggesting the new way. But why do the manual work and not leave that to the automated to

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-29 Thread kellen sunderland
It's more readable because it's concise and it's consistent for many types you're looping over (i.e. primitive arrays, stl iterators, etc all work the same way). It's also useful because it's consistent with other programming languages, making C++ codebases much easier to read for novice and inter

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-29 Thread Naveen Swamy
Kellen, Could you please explain why you think range loops are better and how it improves readability? this is a relatively new feature, many of them are used to the old syntax, shouldn't we leave it for the developers to choose the one that best suits the need and their familiarity. In general I

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-28 Thread Chris Olivier
ok then, my vote is still -1, however, because it’s just adding needless friction for developers imho. On Fri, Sep 28, 2018 at 7:42 AM kellen sunderland < kellen.sunderl...@gmail.com> wrote: > "Range loops aren’t always the most performant way" Do you have an example > where there's a perf differ

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-28 Thread Lin Yuan
+1 Using range-based for-loop whenever possible improves code readability and makes code less prone to human error. I did some preliminary research on Google and did not find any complaint about its performance drawback. Here is one piece from StackOverflow for reference: https://stackoverflow.co

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-28 Thread kellen sunderland
"Range loops aren’t always the most performant way" Do you have an example where there's a perf difference? "In addition, sometimes you want the index. Or maybe you want to iterate backwards, or not start from the first, etc. Maybe you want the iterator because you remove it from the list at the b

Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-28 Thread Chris Olivier
-1 Range loops aren’t always the most performant way. In addition, sometimes you want the index. Or maybe you want to iterate backwards, or not start from the first, etc. Maybe you want the iterator because you remove it from the list at the bottom of the loop Seems like a rule for the sake of

[DISCUSS] Use modernized C++11 range loops uniformly throughout the project

2018-09-28 Thread kellen sunderland
Hello MXNet devs, I'd like to discuss uniformly adopting C++11 range loops in the MXNet project. The benefits I see are: * Improved C++ readability (examples below). * Consistency with other languages. The range-loops are quite similar to loops almost all other programming languages. Given w