openFrameworks icon indicating copy to clipboard operation
openFrameworks copied to clipboard

ofDirectory return wrong value

Open sphaero opened this issue 7 years ago • 1 comments

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.

sphaero avatar Dec 06 '18 14:12 sphaero

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;

igiso avatar Aug 16 '19 19:08 igiso