Unidata / netcdf-cxx4

Official GitHub repository for netCDF-C++ libraries and utilities.
Other
126 stars 49 forks source link

Minimum required C++ standard? #123

Open ZedThree opened 2 years ago

ZedThree commented 2 years ago

In my recent PR #121 I made a change (b18cc3c44e4f15ec5587cec2434981239d27b31e) that requires C++11. It seems the rest of the code is written in C++98 only.

Requiring at least C++11 would allow a pretty decent cleanup and modernisation: using auto to name iterators, using range-for loops, move operators, better support for templates. C++14 probably wouldn't make much difference, but C++17 might allow some more interesting things, though the benefits aren't as immediately obvious as with C++11.

In terms of OS/compiler support, C++11 is completely supported by all major compilers on all current major operating systems, since 2014/2015.

The default system compiler on RHEL7 is slightly too old to support C++14, but the Developer Toolset does have more recent compilers available which support C++17. Other than RHEL7, all other major OSes and compilers support a good fraction of C++17.

I'm very happy to do a C++11 cleanup myself, but I want to check that the maintainers (@WardF) are happy requiring C++11.

Here's the sort of thing switching to C++11 would bring:

@@ -1381,10 +1383,8 @@ void NcGroup::getCoordVar(const string& coordVarName, NcDim& ncDim, NcVar& ncVar

   // search in child groups (makes recursive calls).
   if (check_child_groups) {
-    multimap<string,NcGroup>::iterator it;
-    multimap<string,NcGroup> groups(getGroups());
-    for (it=groups.begin();it!=groups.end();it++) {
-      it->second.getCoordVar(coordVarName,ncDim,ncVar,ChildrenAndCurrent);
+    for (const auto& it : getGroups()) {
+      it.second.getCoordVar(coordVarName,ncDim,ncVar,ChildrenAndCurrent);
       if(!ncDim.isNull()) break;
     }
   }

Not a mind-blowing difference, but still definitely significantly clearer and easier to read.