Just wondering how you guys feel about this sort of thing:
Code:
template<typename T> File<T>* FileSystem::Open( const char* szName, int iMode )
{
FILE* fRead = 0;
File<T>* fCreated = new File<T>;
int iAbsPathLen = strlen( szName ) + strlen( g_szDataRoot ) + 1;
char* szAbsPath = new char[iAbsPathLen];
sprintf( szAbsPath, "%s%s", g_szDataRoot, szName );
if ( iMode & FS_OPEN_READ )
fRead = fopen( szAbsPath, "rb" );
else if ( iMode & FS_OPEN_WRITE )
fRead = fopen( szAbsPath, "wb" );
else
{
Error::GetSingleton().Log( true, "FileSystem::Open: Invalid mode" );
delete[] szAbsPath;
delete fCreated;
return 0;
}
if ( !fRead )
{
Error::GetSingleton().Log( true, "FileSystem::Open: Unable to open '%s'", szAbsPath );
delete[] szAbsPath;
delete fCreated;
return 0;
}
}
To me the two error situations duplicating the same cleanup code seems a bit, well, pointless. I've seen something along the lines of this being done (mainly in the Linux source code):
Code:
template<typename T> File<T>* FileSystem::Open( const char* szName, int iMode )
{
FILE* fRead = 0;
File<T>* fCreated = new File<T>;
int iAbsPathLen = strlen( szName ) + strlen( g_szDataRoot ) + 1;
char* szAbsPath = new char[iAbsPathLen];
sprintf( szAbsPath, "%s%s", g_szDataRoot, szName );
if ( iMode & FS_OPEN_READ )
fRead = fopen( szAbsPath, "rb" );
else if ( iMode & FS_OPEN_WRITE )
fRead = fopen( szAbsPath, "wb" );
else
{
Error::GetSingleton().Log( true, "FileSystem::Open: Invalid mode" );
goto exitFail;
}
if ( !fRead )
{
Error::GetSingleton().Log( true, "FileSystem::Open: Unable to open '%s'", szAbsPath );
goto exitFail;
}
// Return successfully here...
exitFail:
delete[] szAbsPath;
delete fCreated;
return 0;
}
Thoughts?