stop using strings for file path manipulation #1560

Open
ofTheo opened this Issue Sep 6, 2012 · 7 comments

5 participants

@ofTheo

This is sort of a global issue and is being partly addressed by @elliotwoods 's PR for ofDataPath #1523

We should refrain from doing string manipulation on paths and use poco path for everything but actually passing the path to the loader/saver/manipulator .

@bilderbuchi
openFrameworks member

How does that look in the PG? this could possibly reduce the various headaches with paths and double // etc in the PG, right?

@ofZach

this would be a big help for the PG and avoid some headaches. in particular, copyFromTo has been problematic, but overall, I think using a smarter object and avoiding string manipulation everywhere will be a big win.

@arturoc
openFrameworks member

some days ago i changed ofFile and ofDirectory so their comparison operators return the correct value even for different paths if they are actually pointing to the same thing:

ofFile f1("img.png");
ofFile f2("../data/img.png");
f1==f2;

will return true. Poco comparison operators actually don't work for something like that

@elliotwoods

@arturoc - unless we resolve absolute paths for equality

@arturoc
openFrameworks member

yes that's what i did for ofFile and ofDirectory, we should add similar constructors, methods and operators for ofFilePath though, since ofFile actually opens the file

@bilderbuchi bilderbuchi modified the milestone: 0.9.0, 0.9.1 Sep 3, 2014
@arturoc arturoc modified the milestone: 0.11.0, 0.9.3 Nov 22, 2015
@elliotwoods

now we have std::filesystem::path (even though it's actually boost right now), can remove this?
or do we want to make it so that our functions start accepting path as an argument?

@arturoc
openFrameworks member

string autoconverts to path so changing every path argument to std::filesystem::path would be a good idea. the only reason i see to wrap it with something else is that we use the data folder to store assets so we could have some class that did the ofToDataPath thing internally and wrapped filesystem::path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment