Skip to content

Split FileIO MeshLib dependency in LayeredMeshGenerator#1143

Merged
endJunction merged 6 commits intoufz:masterfrom
endJunction:SplitFileIOMeshLibDependency
Apr 19, 2016
Merged

Split FileIO MeshLib dependency in LayeredMeshGenerator#1143
endJunction merged 6 commits intoufz:masterfrom
endJunction:SplitFileIOMeshLibDependency

Conversation

@endJunction
Copy link
Copy Markdown
Member

@endJunction endJunction commented Apr 14, 2016

Move reading of raster files out of layered mesh generator.

Add convenience function to read all raster files given by a list of file names.


/// Reads a vector of rasters given by file names. On error nothing is returned,
/// otherwise the returned vector contains pointers to the read rasters.
boost::optional<std::vector<GeoLib::Raster const*>> readRasters(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it looks bit redundant for me to use boost::optional for std::vector, because one can do the same thing using std::vector::empty().

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree with @endJunction's answer below.

@norihiro-w
Copy link
Copy Markdown
Collaborator

looks good. I have just one minor comment about usage of boost optional. the comment is not critical so go ahead if you don't like it

@endJunction
Copy link
Copy Markdown
Member Author

@norihiro-w I had the empty vector before and it works. Reasons for optional: When looking on the algorithm it is a one-to-one transformation of strings to rasters, and boost::optional indicates if everything was ok. Therefore one can distinguish if an error happened or nothing was to be read at all.

boost::optional<std::vector<GeoLib::Raster const*>> readRasters(
std::vector<std::string> const& raster_paths)
{
if (!allRastersExist(raster_paths)) return boost::none;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Only a note: Actually it is dangerous to first check if files exist, then believe that they exist and just try to read them blindly. The files could have vanished in between. @endJunction, you shouldn't spend time on that now, especially since the problem has not been introduced in this PR. But for the future we should keep in mind that files should be read in one go and not be checked for existence prior to opening.

@chleh
Copy link
Copy Markdown
Collaborator

chleh commented Apr 19, 2016

👍

@endJunction endJunction merged commit e1dc729 into ufz:master Apr 19, 2016
@endJunction endJunction deleted the SplitFileIOMeshLibDependency branch April 19, 2016 10:45
@ogsbot
Copy link
Copy Markdown
Member

ogsbot commented Jun 19, 2020

OpenGeoSys development has been moved to GitLab.

See this pull request on GitLab.

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.

4 participants