1
\$\begingroup\$

This program is supposed to take every embedded XML Schema file in an assembly and generate classes in the csOutPath. The names of the classes and files should match the stored schema and be put in the Target Assembly's "GeneratedClasses" folder.

Only requirement is that classes must have their namespace and folder location be:

{targetAssembly}.GeneratedClasses.{SubFolderTakenFromEmbeddedResource}.{ResourceNameStrippedOfIllegalChars} 

I would prefer to not create local temp files as I'm having problems with file locking, but I'm using the XSD.exe tool. I also have several parallel processes to speed it up but it is still a bit slow (this program runs on every build of the other project).


internal class Program { #region Constants private static readonly Assembly assembly = Assembly.Load("AssemblyName"); private const string basepath = "TMP_SCHEMA_OUTPUT_PATH"; private const string csOutPath = "D:\\_Workspace\\Integrations\\XmlHelper\\GeneratedClasses"; private static readonly string warningMessage = "// DO NOT MODIFY THIS FILE. Instead modify how it\'s build " + $"in {typeof(Program).Assembly.GetName().Name}.\r\n" + "// Any modifications are overwritten in the pre-build event."; #endregion private static void Main() { /* ----------------------- Build Schemas ----------------------- */ var schemasBuilt = BuildSchemas(); /* ------------------ Run the XSD.exe process ------------------ */ Parallel.ForEach(schemasBuilt, schema => { var name = Path.GetFileNameWithoutExtension(schema); var baseName = assembly.GetName().Name; /* ------------------- Build namespaces -------------------- */ var process = new Process { StartInfo = { WindowStyle = ProcessWindowStyle.Hidden, UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true, WorkingDirectory = Directory.GetCurrentDirectory(), CreateNoWindow = true, FileName = "xsd.exe", Arguments = "/order " + $"{schema} " + "/c " + $"/out:{basepath} " + $"/namespace:{baseName}.GeneratedClasses.{name}" }, EnableRaisingEvents = true }; process.Start(); Console.WriteLine(process.StandardOutput.ReadToEnd()); Console.WriteLine(process.StandardError.ReadToEnd()); }); /* --------------------------------------------------------------*/ /* ---------------------- Write the files ---------------------- */ BuildCsFiles(new DirectoryInfo(basepath) .EnumerateFiles() .Where(x => x.Extension.Contains("cs")) .Select(y => y.FullName).ToList()); /* -------------------------- Cleanup -------------------------- */ Directory.Delete(basepath, true); /* --------------------------------------------------------------*/ } private static void BuildCsFiles(IEnumerable<string> generatedCsFiles) { foreach (var generatedPath in generatedCsFiles) { var firstLine = true; var name = Path.GetFileName(generatedPath); var file = Path.Combine(csOutPath, name.Replace("_", "\\")); Directory.CreateDirectory(Path.GetDirectoryName(file)); /* ---------------- Write and update files ----------------- */ using (var reader = new StreamReader(generatedPath)) using (var fileStream = new FileStream(file, FileMode.Create)) using (var writer = new StreamWriter(fileStream)) { string tmpLine; while ((tmpLine = reader.ReadLine()) != null) { /* ----------- Any class changes go here ----------- */ writer.WriteLine(tmpLine); if (!firstLine) continue; writer.WriteLine(warningMessage); firstLine = false; } writer.Flush(); } } } /// <summary> /// Builds Schemas. /// </summary> /// <returns>A list of schemas by their names.</returns> private static IEnumerable<string> BuildSchemas() { var schemas = new ConcurrentBag<string>(); var relevantResources = assembly.GetManifestResourceNames() .Where(x => x.Contains(".xsd")); /* ---------------------- Write the files ---------------------- */ Parallel.ForEach(relevantResources, resource => { Directory.CreateDirectory(basepath); // Schemas are stored in NameSpace.Schemas.*.<schemaName> string key = Regex.Replace(resource, "(.*Schemas\\.|\\(.*)", String.Empty); var tmpOutPath = Path.Combine(basepath, key + ".xsd"); using (var s = assembly.GetManifestResourceStream(resource)) using (var fileStream = WaitForFile(tmpOutPath, FileMode.Create, FileAccess.ReadWrite, FileShare.Write)) { if (fileStream == null) { Console.WriteLine($"Unable to unlock {tmpOutPath} after 10 attempts. Exiting..."); throw new IOException($"Unable to unlock {tmpOutPath} after 10 attempts."); } s?.Seek(0, SeekOrigin.Begin); s?.CopyTo(fileStream); schemas.Add(tmpOutPath); } }); return schemas; } /// <summary> /// Waits for the file to be available 10 times before throwing an error /// </summary> /// <returns></returns> static FileStream WaitForFile(string fullPath, FileMode mode, FileAccess access, FileShare share) { for (var numTries = 0; numTries < 10; numTries++) { FileStream fs = null; try { fs = new FileStream(fullPath, mode, access, share); return fs; } catch (IOException) { fs?.Dispose(); Thread.Sleep(50); } } return null; } } 
\$\endgroup\$
1
  • 2
    \$\begingroup\$Welcome to Code Review! Please take a look at the FAQ about asking questions. Do you have any particular concerns you want reviewers to address?\$\endgroup\$
    – Null
    CommentedDec 6, 2017 at 17:24

1 Answer 1

1
\$\begingroup\$

The code is not bad but it could be better. These are my suggestions:


/* ---------------------- Write the files ---------------------- */ BuildCsFiles(new DirectoryInfo(basepath) .EnumerateFiles() .Where(x => x.Extension.Contains("cs")) .Select(y => y.FullName).ToList()); 

Unless you are really searching for any file extensions that conatin the cs using Equals would be a better idea:

.Equals(".cs", StringComparison.OrdinalIgnoreCase) 

According to MSDN

The Extension property returns the FileSystemInfo extension, including the period (.). For example, for a file c:\NewFile.txt, this property returns ".txt".


private static void BuildCsFiles(IEnumerable<string> generatedCsFiles) 

The parameter name should be generatedCsFileNames. I couldn't figure out at first what's going on here and why you are passing generated-cs-files to a method doing virtually the same thing.


using (var s = assembly.GetManifestResourceStream(resource)) using (var fileStream = WaitForFile(tmpOutPath, FileMode.Create, FileAccess.ReadWrite, FileShare.Write)) { if (fileStream == null) { Console.WriteLine($"Unable to unlock {tmpOutPath} after 10 attempts. Exiting..."); throw new IOException($"Unable to unlock {tmpOutPath} after 10 attempts."); } s?.Seek(0, SeekOrigin.Begin); s?.CopyTo(fileStream); schemas.Add(tmpOutPath); } 

You should not throw this exception here. It should be the reponsibility of WaitForFile. If you change 10 to something else you would need to adjust two methods instead of just one where you could use a constant anyway.

Btw. you have such good names everywhere. Why did you decide to use just the letter s here?


I also think you use a very strange and difficult to read formatting style. For example this:

var relevantResources = assembly.GetManifestResourceNames() .Where(x => x.Contains(".xsd")); 

is much easier to read as

var relevantResources = assembly .GetManifestResourceNames() .Where(x => x.Contains(".xsd")); 

or alternatively as a query:

var relevantResources = from resourceName in assembly.GetManifestResourceNames() where resourceName.EndsWith(".xsd", StringComparison.OrdinalIgnoreCase) select resourceName; 

You also might want use a case insensitive search and look at the end of the resource name and not whether it contains the string. What if you named it Something.xsd.txt. Your predicate would find it... unless this is what you want.


Then I would rewrite

BuildCsFiles(new DirectoryInfo(basepath) .EnumerateFiles() .Where(x => x.Extension.Equals(".cs", StringComparison.OrdinalIgnoreCase)) .Select(y => y.FullName).ToList()); 

as two statements:

var csFileNames = new DirectoryInfo(basepath) .EnumerateFiles() .Where(x => x.Extension.Equals(".cs", StringComparison.OrdinalIgnoreCase)) .Select(y => y.FullName); BuildCsFiles(csFileNames); 

and if you already use the lazy EnumerateFiles why do you make it un-lazy with ToList? This is not necessary.


var name = Path.GetFileName(generatedPath); var file = Path.Combine(csOutPath, name.Replace("_", "\\")); 

These two lines are confusing. name does not say what kind of a name this is and file makes me think it's a file of some kind but it's actually a path. I know this as fileName and fullName. You already use this style with generatedCsFileNames and .net is using it here .Select(y => y.FullName);. Try to be consistant.

\$\endgroup\$

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.