The Wayback Machine - https://web.archive.org/web/20200525095246/https://github.com/openframeworks/openFrameworks/issues/6190
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ofDirectory return wrong value #6190

Open
sphaero opened this issue Dec 6, 2018 · 1 comment
Open

ofDirectory return wrong value #6190

sphaero opened this issue Dec 6, 2018 · 1 comment

Comments

@sphaero
Copy link
Contributor

@sphaero sphaero commented Dec 6, 2018

In the documentation it says

bool ofDirectory::createDirectory(const filesystem::path &dirPath, bool bRelativeToData=true, bool recursive=false)

Returns: true if directory was created successfully

However when I look in the code it will always return true:

bool ofDirectory::createDirectory(const std::filesystem::path& _dirPath, bool bRelativeToData, bool recursive){
	auto dirPath = _dirPath;

	if(bRelativeToData){
		dirPath = ofToDataPath(dirPath);
	}
	
	// on OSX,std::filesystem::create_directories seems to return false *if* the path has folders that already exist
	// and true if it doesn't
	// so to avoid unnecessary warnings on OSX, we check if it exists here:
	
	bool bDoesExistAlready = ofDirectory::doesDirectoryExist(dirPath,false);
	
	if (!bDoesExistAlready){
		
		bool success = false;
		try{
			if(!recursive){
				success = std::filesystem::create_directory(dirPath);
			}else{
				success = std::filesystem::create_directories(dirPath);
			}
		} catch(std::exception & except){
			ofLogError("ofDirectory") << "createDirectory(): couldn't create directory \"" << dirPath << "\": " << except.what();
			return false;
		}
		return success;
	}
	
	// no need to create it - it already exists.
	return true;
}

Basically when read as bDoesExistAlready is true the return value should be false.

However this gets tricky if the creation of the directory fails, it will then also return false which would then have different meaning.

@igiso
Copy link

@igiso igiso commented Aug 16, 2019

I think it's better if the documentation was just changed to:

Returns: true if directory was created successfully or already exists

instead of touching the core
You want this to return false explicitly when it fails to create, (for example in a read only path etc) because you have no other way to know case fail creation, you can know if the directory already existed with:

bool ofDirectory::exists() const;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.