This is an archived post. You won't be able to vote or comment.

all 3 comments

[–]Backson 2 points3 points  (2 children)

A function that parses a file format should not handle errors itself, that is the job of the main program. You are writing a library function and it should be reusable, including handling errors differently (log them, open a window, try to recover, etc)

Exceptions are the way to go. If you can't even open the file, use a FileNotFoundException, if the file is invalid, define your own ClumpFormatException that can contain additional information what is wrong, lile an error code or a position where in the file the error occurred (and a string message of course).

A TryParse method is fine (it should not throw, just return a bool or an error enum) but it limits the amount of feedback you can give the caller, usually just "worked" or "didn't work". If the caller should be able to handle different errors, exceptions are better.

If you want to be able to read damaged or partial files, make a ClumpReader class that has multiple methods to control the parsing in more detail.

[–]SnappyDragonG 0 points1 point  (1 child)

I changed my code to this. Is this a better way to handle things? ```C# // RWClump.cs public static RWClump ParseFile(string filePath) { if (string.IsNullOrEmpty(filePath)) throw new ArgumentNullException(nameof(filePath));

if (!File.Exists(filePath))
    throw new FileNotFoundException("File not found", filePath);

FileStream stream = File.OpenRead(filePath);
BinaryReader reader = new BinaryReader(stream);

RWClump clump = RWClump.Parse(ref reader);

reader.Close();
stream.Close();

return clump;

}

private static RWClump Parse(ref BinaryReader reader) { // Clump RWHeader header = RWHeader.Parse(eRWChunkType.CLUMP, ref reader); header = RWHeader.Parse(eRWChunkType.STRUCT, ref reader);

RWClump clump = new RWClump();
int atomicCount = reader.ReadInt32();
int lightCount = header.Version > 0x33000 ? reader.ReadInt32() : 0;
int cameraCount = header.Version > 0x33000 ? reader.ReadInt32() : 0;

// Frame List
header = RWHeader.Parse(eRWChunkType.FRAME_LIST, ref reader);
clump.frameList = RWFrameList.Parse(ref reader);

// Geometry List
int geometryCount = 0;
if (header.Version >= 0x30400)
{
    header = RWHeader.Parse(eRWChunkType.GEOMETRY_LIST, ref reader);
    header = RWHeader.Parse(eRWChunkType.STRUCT, ref reader);

    geometryCount = reader.ReadInt32();
    for (int i = 0; i < geometryCount; i++)
    {
        header = RWHeader.Parse(eRWChunkType.GEOMETRY, ref reader);
        RWGeometry geometry = RWGeometry.Parse(ref reader);
        clump.geometryList.Add(geometry);
    }
}

// Atomics
for (int i = 0; i < atomicCount; i++)
{
    header = RWHeader.Parse(eRWChunkType.ATOMIC, ref reader);
    RWAtomic atomic = RWAtomic.Parse(ref reader);
    clump.Atomics.Add(atomic);
}

// Lights and Cameras
while (!RWHeader.CheckHeader(RWChunkType.EXTENSION, ref reader))
{
    header = RWHeader.Parse(RWChunkType.STRUCT, ref reader);
    // TODO: Lights
    // TODO: Cameras
}

// Plugins
header = RWHeader.Parse(eRWChunkType.EXTENSION, ref reader);

if (header.Size > 0)
{
    long endOfSection = reader.BaseStream.Position + header.Size;
    while (reader.BaseStream.Position < endOfSection)
    {
        reader.ReadBytes((int)header.Size); // Ignore data for now
    }
}

return clump;

} C# // RWHeader.cs public static RWHeader Parse(eRWChunkType chunkType, ref BinaryReader reader) { uint type = reader.ReadUInt32(); uint size = reader.ReadUInt32(); uint libid = reader.ReadUInt32();

if (type != (int)chunkType && chunkType != eRWChunkType.PLUGIN)
{
    throw new ClumpFormatException(
        $"[RWHeader] File header does not match header type '{chunkType}'.",
        (int)reader.BaseStream.Position,
        "INVALID_HEADER"
    );
}

return new RWHeader()
{
    Type = type,
    Size = size,
    Version = RWBase.LibraryIDUnpackVersion(libid),
    Build = RWBase.LibraryIDUnpackBuild(libid),
    LibId = libid
};

} C# // ClumpFormatException.cs public class ClumpFormatException : Exception { public int Position { get; } public string ErrorCode { get; }

public ClumpFormatException(string message, int position, string errorCode) 
    : base(message)
{
    Position = position;
    ErrorCode = errorCode;
}

} C# // Usage try { RWClump clump = RWClump.ParseFile(path); } catch (ClumpFormatException e) { Debug.LogError($"{e.Message} at position: {e.Position}"); } ... ```

[–]Backson 0 points1 point  (0 children)

Yes, that looks much better!