Post-cpp11 conversion: remaining refactoring
Now that vapour is on cpp11 (#xxx), there are some follow-up improvements worth doing. None are urgent — the package is clean and tested — but they'd reduce complexity for future work.
Split gdallibrary.h
At 784 lines it mixes vector layer utilities, CRS functions, driver listing, field allocation, and legacy read functions. Could split into:
gdal_layer_utils.h — gdal_layer(), with_ogr_layer(), open_ogr_dataset(), force_layer_feature_count(), gdal_layer_geometry_name(), gdal_layer_extent()
gdal_crs.h — gdal_proj_to_wkt(), gdal_crs_is_lonlat(), gdal_projection_info()
gdal_drivers.h — gdal_list_drivers(), gdal_driver(), gdal_layer_names(), gdal_vsi_list(), gdal_version(), proj_version(), gdal_set_config_option(), gdal_get_config_option()
gdal_field_alloc.h — allocate_fields_list()
Deduplicate field-reading switch logic
layer_read_fields_ia and layer_read_fields_fa in gdalgeometry.h have a cut-down field type switch (only OFTInteger/OFTReal/OFTString) while layer_read_fields_ij has the full switch including DateTime, Binary, etc. The ia/fa variants are marked FIXME copy the wider field support from sf. Extract the per-field-type write logic into a shared helper function.
Remove legacy gdal_read_fields / gdal_read_geometry
The gdal_read_fields() and gdal_read_geometry() functions in gdallibrary.h are the old limit_n/skip_n implementations. The gdalgeometry.h layer_read_fields_ij/layer_read_geom_ij functions (called via read_fields_gdal_cpp/read_geometry_gdal_cpp in 00_geometry.cpp) now handle this. Check if the old functions are still called anywhere; if not, remove them.
Add as_cstr() helper
The std::string(x[0]).c_str() pattern appears ~50 times across the headers. A small helper in common_vapour.h would clean this up:
inline const char* as_cstr(cpp11::r_string x) {
return CHAR(static_cast<SEXP>(x));
}
// usage: GDALOpen(as_cstr(dsn[0]), GA_ReadOnly)
Post-cpp11 conversion: remaining refactoring
Now that vapour is on cpp11 (#xxx), there are some follow-up improvements worth doing. None are urgent — the package is clean and tested — but they'd reduce complexity for future work.
Split gdallibrary.h
At 784 lines it mixes vector layer utilities, CRS functions, driver listing, field allocation, and legacy read functions. Could split into:
gdal_layer_utils.h—gdal_layer(),with_ogr_layer(),open_ogr_dataset(),force_layer_feature_count(),gdal_layer_geometry_name(),gdal_layer_extent()gdal_crs.h—gdal_proj_to_wkt(),gdal_crs_is_lonlat(),gdal_projection_info()gdal_drivers.h—gdal_list_drivers(),gdal_driver(),gdal_layer_names(),gdal_vsi_list(),gdal_version(),proj_version(),gdal_set_config_option(),gdal_get_config_option()gdal_field_alloc.h—allocate_fields_list()Deduplicate field-reading switch logic
layer_read_fields_iaandlayer_read_fields_fain gdalgeometry.h have a cut-down field type switch (only OFTInteger/OFTReal/OFTString) whilelayer_read_fields_ijhas the full switch including DateTime, Binary, etc. The ia/fa variants are markedFIXME copy the wider field support from sf. Extract the per-field-type write logic into a shared helper function.Remove legacy gdal_read_fields / gdal_read_geometry
The
gdal_read_fields()andgdal_read_geometry()functions in gdallibrary.h are the old limit_n/skip_n implementations. The gdalgeometry.hlayer_read_fields_ij/layer_read_geom_ijfunctions (called viaread_fields_gdal_cpp/read_geometry_gdal_cppin 00_geometry.cpp) now handle this. Check if the old functions are still called anywhere; if not, remove them.Add
as_cstr()helperThe
std::string(x[0]).c_str()pattern appears ~50 times across the headers. A small helper in common_vapour.h would clean this up: