4
\$\begingroup\$

The reasons I create this program is because we had an issue of XML elements showing up with older assembly versions in, which were duplicate elements of the same element with the new assembly version in. Here is an example of the problematic files we have:

<data attribute="value" >
    <value>Version="dot_net_4_assembly_version_number", <!--This element is fine--></value>
</data>

<data attribute="anotherValue" >
    <value>Version="dot_net_2_assembly_version_number", "identical_data_here"</value>
</data>

<data attribute="anotherValue" >
    <value>Version="dot_net_4_assembly_version_number", "identical_data_here"</value>
</data>

So my program will be given a directory, and loop through all files in all sub-directories of that directory, and then if it matches with the regex to find the references to the older assemblies, it will remove that xml element.

Here is the code:

class Program
{
    static int totalChangedFilesCount;
    static int totalRemovedDotNetTwoReferences;

    static void Main(string[] args)
    {
        var allFilePaths = new List<string>();
        allFilePaths = GetAllFilePaths("C:/Temp/SolutionToTest");

        foreach(var filePath in allFilePaths)
        {
            RemoveOldAssemblyReferencesFromFile(filePath);
        }

        Console.WriteLine($"Total files changed: {totalChangedFilesCount}");
        Console.WriteLine($"Total .NET 2 refernces removed: {totalRemovedDotNetTwoReferences}");

        Console.ReadLine();

    }

    private static List<string> GetAllFilePaths(string sourceDirectory, string filePattern = "*.*")
    {
        var filePaths = new List<string>();

        try
        {
            foreach (string file in Directory.EnumerateFiles(sourceDirectory, filePattern, SearchOption.AllDirectories))
            {
                filePaths.Add(file);
            }
        }
        catch (Exception ex)
        {
            Console.WriteLine($"Error: {ex.Message}");
        }

        return filePaths;

    }

    static void RemoveOldAssemblyReferencesFromFile(string filePath)
    {
        string xmlToClean = string.Empty;

        try
        {
            if (string.IsNullOrEmpty(filePath))
            {
                Console.WriteLine("No file path specified");
                return;
            }
            else
            {
                xmlToClean = File.ReadAllText(filePath);
            }
        }
        catch (Exception ex)
        {
            Console.WriteLine($"Error: {ex.Message}");
        }

        var regex = new Regex(@"<data.*\s*.*Version=1.0.0.5.*\s*</data>");
        var matches = regex.Matches(xmlToClean);

        if (matches.Count == 0)
        {
            Console.WriteLine($"No matches found in {Path.GetFileName(filePath)}");
            return;
        }

        string newXml = string.Empty;
        foreach (var match in matches)
        {
            var str = match.ToString();
            if (xmlToClean.Contains(str))
            {
                newXml = xmlToClean.Replace(str, "\r\n");
                totalRemovedDotNetTwoReferences++;
            }
        }

        File.WriteAllText(filePath, newXml);
        Console.WriteLine($"Finished clearing {Path.GetFileName(filePath)} of old assembly referenes");
        totalChangedFilesCount++;
    }
}

A couple of things to note:

  • This is before the code is going to be fully executed on the actual code, up until now I have simply copied the solution into "C:/Temp/SolutionToTest", and when I come to actually running the program this will obviously change.
  • The program hasn't been made to have any sort of UI or interactivity i.e. I haven't allowed it to simply be ran, then the user could input file paths or patterns etc, because A) this program probably will only be used once and B) that wouldn't be very hard to implement (probably).
  • Some assumptions have been made such as assuming that the sourceDirectory is correct and actually has files in anyways.
  • The perfect grammar or vocabulary isn't that important for the program.
  • This is a simple Console Program, no complex GUI needed or required. Doesn't need to be extensively designed code to follow any particular pattern or anything, this is mainly just in case I've done something glaringly bad.

Edit:

Just for proof, here is the regex working: working program using regex

\$\endgroup\$
  • 1
    \$\begingroup\$ If a single-use throwaway program does what it's intended to do, that seems like good enough to me. Hardly worth getting it reviewed, unless you're trying to apply lessons here to improve your general C# programming skills? Is there anything in particular you'd like reviewers to focus on? \$\endgroup\$ – Cody Gray Jan 9 '17 at 16:24
  • 1
    \$\begingroup\$ Are you sure this is even working code? \$\endgroup\$ – paparazzo Jan 9 '17 at 20:00
  • \$\begingroup\$ Working with .resx Files Programmatically - [..] XML resource (.resx) files must consist of well-defined XML,[..] \$\endgroup\$ – t3chb0t Jan 10 '17 at 8:39
  • \$\begingroup\$ Please remove the edit because now your question qualfies to be closed for broken code. \$\endgroup\$ – t3chb0t Jan 10 '17 at 8:41
  • \$\begingroup\$ @t3chb0t The thing is, my code works and when I run it, it removes what I want from the files. Manually checking the files also proves the code works. \$\endgroup\$ – Danny Goodall Jan 10 '17 at 8:43
3
\$\begingroup\$

You can replace this line with a LINQ expression

foreach (string file in Directory.EnumerateFiles(sourceDirectory, filePattern, SearchOption.AllDirectories))
{
    filePaths.Add(file);
}

Like this :

filePaths.AddRange(Directory.EnumerateFiles(sourceDirectory, filePattern, SearchOption.AllDirectories));

Which is shorter and probably more performant as it uses .AddRange() which avoid additional allocations which foreach would cause.

But I don't see any reason why would you create a list that value (new List<>() - empty) might not be used at all. You should return empty list only if you cant return actual results :

private static IEnumerable<string> GetAllFilePaths(string sourceDirectory, string filePattern = "*.*")
{
    try
    {
        return Directory.EnumerateFiles(sourceDirectory, filePattern, SearchOption.AllDirectories);
    }
    catch (Exception ex)
    {
        Console.WriteLine($"Error: {ex.Message}");
    }
    return new List<string>(); ;
}

Note that I swapped the List<string> return parameter with IEnumerable<string>

Additionally you might consider adding a check against null/empty string for the path, just to avoid the performance drop of catching the exception, as that will be the most common error there I suppose.

var allFilePaths = new List<string>();
allFilePaths = GetAllFilePaths("C:/Temp/SolutionToTest");

foreach (var filePath in allFilePaths)
{
    RemoveOldAssemblyReferencesFromFile(filePath);
}

There is no point of allocation memory for new List<string>() just to instantly override it on the next line. You can shorten that to :

var allFilePaths = GetAllFilePaths("C:/Temp/SolutionToTest");

foreach (var filePath in allFilePaths)
{
    RemoveOldAssemblyReferencesFromFile(filePath);
}

Or if you want to, you can put the collection in the foreach too :

foreach (var filePath in GetAllFilePaths("C:/Temp/SolutionToTest"))
{
    RemoveOldAssemblyReferencesFromFile(filePath);
}
| improve this answer | |
\$\endgroup\$
6
\$\begingroup\$

I cannot look at it. You treat a XML like it was a normal text file. This is insane.

You should parse it with the XDocument.Load, find all the data and value elements with LINQ or XPath and then check the value if it matches the pattern and if necessary call XElement.Remove and resave the file.


Btw. this regex doesn't work

<data.*\s*.*Version=1.0.0.5.*\s*</data>

The . in the version number need to be escaped as well as the /.

Using the .* is probably also killing the performance: see Why Using the Greedy .* in Regular Expressions Is Almost Never What You Actually Want

| improve this answer | |
\$\endgroup\$
  • 2
    \$\begingroup\$ Parsing XML with regex is probably worse than doing it to HTML, too. \$\endgroup\$ – Bob Jan 10 '17 at 0:34
  • \$\begingroup\$ Strange, the comment about the regex not working makes sense and from what you have said I would have guessed that it wouldn't work, although somehow the program didn't work. I'm playing about with the XML idea now, thanks for the advice. \$\endgroup\$ – Danny Goodall Jan 10 '17 at 9:13
1
\$\begingroup\$

Although I agree with @CodyGray comment, I think you can slightly improve the code performance wise by not creating the Regex for each file but to have it as a static variable which you should create with the Compiled overload like so

private static Regex regex = new Regex(@"<data.*\s*.*Version=1.0.0.5.*\s*</data>", RegexOptions.Compiled);

See: https://stackoverflow.com/questions/513412/how-does-regexoptions-compiled-work

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ This regex won't work. The dots in the version need to be escaped, right? \$\endgroup\$ – t3chb0t Jan 9 '17 at 17:14
  • 1
    \$\begingroup\$ @t3chb0t then the original doesn't work as well because I just copied it. \$\endgroup\$ – Heslacher Jan 9 '17 at 17:15
0
\$\begingroup\$

REGEX is not the proper tool to process XML as covered by t3chb0t.

Why are you generating the whole List then process one by one? Just enumerate the files and process them at that time. If you use FileInfo rather than File you can use any of the Open methods to open the text file directly.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.