Created
October 12, 2020 17:04
-
-
Save matlus/a6c2b3007261d0b155b1eea034135acb to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| /* | |
| * 1. The local variable "schema" in the Main() method should be called xmlSchema | |
| * 2. The method GetSchemaDocumentFromFile() should be called GetXmlSchemaFromFile() | |
| * 3. There should be only one argument to method GetPpapiSchemaDocumentFromFile() and it should be called schemaFilePathAndName. | |
| * There is no need for this method to have to know what applicationRootFolder means and how to constructor a full file path | |
| * and name ofthe file it needs. The caller should be responsible for that. | |
| * 4. The method AddDataElementToSchemaIfNotPresent() is modifying the document. To show your intent (make it obvious the document | |
| * is going to be potentially modified) should do one of the following | |
| * a. Either make the argument a "ref" argument | |
| * b. Return the modified document from this method. | |
| * C# Guidelines state they ref and out should be avoided. So #b above is the correct choice. * | |
| * 5. MyApplicatioNameSchemaException - remove extra lines at the top | |
| * 6. MyApplicatioNameSchemaException should be sealed and the protected constructor should be made private | |
| * 7. In the method GetSchemaDocumentFromFile() try-catch nested inside a using block. This results in a try-catch nested inside | |
| * a try-finally (bit of a performance hit. This should be changed to a try-catch-finally instead | |
| * 8. In the method GetSchemaDocumentFromFile() the local variable "stream" should be called fileStream | |
| * 9. In the method GetSchemaDocumentFromFile() the local variable "schema" should be called xmlSchema | |
| * 10. In the method GetSchemaDocumentFromFile() the call to XmlSchema.Read(), the second argument - use named arguments here since just "null" doesn't help | |
| * 11. The call to the medthod - AddDataElementToSchemaIfNotPresent and the return statement should be in the catch block | |
| * 12. When throwing an Exception in a catch block, always include the original exception as the inner exception | |
| * 13. The Exception message should contain more contextual information. Simply providing the original exception message without augmenting it has no value add. | |
| * | |
| * **** Exceptions Handling **** | |
| * If you're handling exceptions there needs to be a value-add. Catching one type of exception to throw another type | |
| * has no real benefit. Also, you MUST include the original exception as the inner exception (No exceptions to this rule) | |
| * Additionally, code like this (the method GetPpapiSchemaDocumentFromFile()) is generally deep in the guts of the system | |
| * the other parts of the system (or the calling service or UI) have no knowledge of a "File" so if they see a file related | |
| * exception, not only are you leaking the implementation details of the system but it generally has no meaning to the outter | |
| * parts of your sytem. | |
| * So, if you're catching one type of exception only to throw another type. Then you MUST do the following | |
| * A. Account for all possible exception types for the particular code - in this case IOException, XmlException, XmlSchemaException | |
| * A. Throw a specific type of exception for the various scenarios/possibilities | |
| * B. Provide detailed context information in your exception, in addition to the original exception message | |
| * C. Provide information to the caller that will help them resolve the issue | |
| */ | |
| internal static class Program | |
| { | |
| static void Main(string[] args) | |
| { | |
| /* | |
| * We have an XM Document and now we're attempting to validate the schema of | |
| * this document against an Xml Schema document. The next line of code is | |
| * simply attempting to retrieve the Xml Schema document. | |
| * The applicationRootFolder argument is determined at runtime (dependant upon the runtime context; Console app or Web App etc.) | |
| * The fileName argument comes from the config file. The concatenation of the 2 is the full path and file name | |
| * The arguments to the method below have been hard coded for the sake brevity | |
| */ | |
| var xmlSchema = GetXmlSchemaFromFile(@"..\..\XMLSchema1.xsd"); | |
| /* | |
| * Now that we have a schema document, we can validate our XmlDocument against the schema | |
| * This part of the code has been intentionally elided | |
| */ | |
| } | |
| private static XmlSchema GetXmlSchemaFromFile(string xmlschemaFile) | |
| { | |
| FileStream fileStream = null; | |
| try | |
| { | |
| fileStream = File.OpenRead(xmlschemaFile); | |
| var xmlSchema = XmlSchema.Read(fileStream, validationEventHandler: null); | |
| return AddDataElementToSchemaIfNotPresent(xmlSchema); | |
| } | |
| catch (IOException e) | |
| { | |
| throw new MyApplicatioNameSchemaException($"The Xml Schema document was not found in the path: {Path.GetFullPath(xmlschemaFile)}. \r\n {e.Message}", e); | |
| } | |
| catch (XmlException e) | |
| { | |
| throw new MyApplicatioNameSchemaException($"The Xml Schema document found in the path: {xmlschemaFile} is not a valid Xml document. \r\n {e.Message}", e); | |
| } | |
| catch (XmlSchemaException e) | |
| { | |
| throw new MyApplicatioNameSchemaException($"The Xml Schema document found in the path: {xmlschemaFile} is not a valid Xml Schema document. \r\n {e.Message}", e); | |
| } | |
| finally | |
| { | |
| fileStream?.Close(); | |
| } | |
| } | |
| private static XmlSchema AddDataElementToSchemaIfNotPresent(XmlSchema schema) | |
| { | |
| //// The implementation of this method has been intentionally elided | |
| return schema; | |
| } | |
| } | |
| [Serializable] | |
| public sealed class MyApplicatioNameSchemaException : Exception | |
| { | |
| public MyApplicatioNameSchemaException() { } | |
| public MyApplicatioNameSchemaException(string message) : base(message) { } | |
| public MyApplicatioNameSchemaException(string message, Exception inner) : base(message, inner) { } | |
| private MyApplicatioNameSchemaException( | |
| System.Runtime.Serialization.SerializationInfo info, | |
| System.Runtime.Serialization.StreamingContext context) : base(info, context) { } | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment