Skip to content

Conversation

spywo
Copy link

@spywo spywo commented Oct 2, 2025

Like other languages, the .NET should have its own exception defined for project.

Like other languages, the .NET should have its own exception defined for project.
@gangatp gangatp requested a review from Copilot October 7, 2025 10:15
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a custom exception class for C# bindings in the project, replacing generic Exception usage with a project-specific exception type. The implementation includes error code mapping, human-readable error descriptions, and comprehensive documentation.

  • Adds a custom {NameSpace}Exception class with error code and message properties
  • Implements error name and description lookup functionality for better error reporting
  • Updates error throwing logic to use the new custom exception instead of generic Exception

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +719 to +722
w.Writeln(" private static string FormatMessage(int errorCode, string errorMessage)")
w.Writeln(" {")
w.Writeln(" string errorName = GetErrorName(errorCode);")
w.Writeln(" string errorDesc = GetErrorDescription(errorCode);")
Copy link

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FormatMessage method creates duplicate code by repeating the same switch logic found in ErrorName and ErrorDescription properties. Consider reusing the existing properties instead of calling separate GetErrorName and GetErrorDescription methods.

Copilot uses AI. Check for mistakes.

Comment on lines +730 to +751
w.Writeln(" {")
w.Writeln(" switch (errorCode)")
w.Writeln(" {")
w.Writeln(" case 0: return \"SUCCESS\";")
for _, errorDef := range component.Errors.Errors {
w.Writeln(" case %d: return \"%s\";", errorDef.Code, errorDef.Name)
}
w.Writeln(" default: return \"UNKNOWN\";")
w.Writeln(" }")
w.Writeln(" }")
w.Writeln("")
w.Writeln(" private static string GetErrorDescription(int errorCode)")
w.Writeln(" {")
w.Writeln(" switch (errorCode)")
w.Writeln(" {")
w.Writeln(" case 0: return \"success\";")
for _, errorDef := range component.Errors.Errors {
w.Writeln(" case %d: return \"%s\";", errorDef.Code, errorDef.Description)
}
w.Writeln(" default: return \"unknown error\";")
w.Writeln(" }")
w.Writeln(" }")
Copy link

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GetErrorName and GetErrorDescription methods duplicate the switch logic already present in the ErrorName and ErrorDescription properties. This creates code duplication and maintenance overhead. These static methods are unnecessary since the instance properties provide the same functionality.

Suggested change
w.Writeln(" {")
w.Writeln(" switch (errorCode)")
w.Writeln(" {")
w.Writeln(" case 0: return \"SUCCESS\";")
for _, errorDef := range component.Errors.Errors {
w.Writeln(" case %d: return \"%s\";", errorDef.Code, errorDef.Name)
}
w.Writeln(" default: return \"UNKNOWN\";")
w.Writeln(" }")
w.Writeln(" }")
w.Writeln("")
w.Writeln(" private static string GetErrorDescription(int errorCode)")
w.Writeln(" {")
w.Writeln(" switch (errorCode)")
w.Writeln(" {")
w.Writeln(" case 0: return \"success\";")
for _, errorDef := range component.Errors.Errors {
w.Writeln(" case %d: return \"%s\";", errorDef.Code, errorDef.Description)
}
w.Writeln(" default: return \"unknown error\";")
w.Writeln(" }")
w.Writeln(" }")

Copilot uses AI. Check for mistakes.

Copy link
Collaborator

@gangatp gangatp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you

Before merging please update the RTTI example for the C# binding, and if possible create an example for reading the exception in it.

@spywo spywo requested a review from gangatp October 10, 2025 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants