3
\$\begingroup\$

I am trying to eliminate unwanted namespaces, and I know there's a better way to do this. This code works, but it's remedial and redundant.

string sPattern = "xmlns:d5p1=\"http://www.w3.org/2001/XMLSchema-instance\" d5p1:nil=\"true\"";
string sPattern2 = "d5p1:nil=\"true\" xmlns:d5p1=\"http://www.w3.org/2001/XMLSchema-instance\"";
string sPattern3 = "xmlns:d2p1=\"http://www.w3.org/2001/XMLSchema-instance\" d2p1:nil=\"true\"";

string[] fString = new string[sentences.Count()];
//foreach (string str in sentences)
for(int i=0; i < sentences.Count(); i++)
{
    Console.WriteLine("{0,24}", sentences[i]); // lines 0-24.
    if(System.Text.RegularExpressions.Regex.IsMatch(sentences[i], sPattern, System.Text.RegularExpressions.RegexOptions.IgnoreCase))
    {
        Console.WriteLine(" (match for '{0}' found)", sPattern);
        eString = sentences[i].Replace(sPattern, "");

    }
    else if(System.Text.RegularExpressions.Regex.IsMatch(sentences[i], sPattern2, System.Text.RegularExpressions.RegexOptions.IgnoreCase))   
    {
        Console.WriteLine(" (match for '{0}' found)", sPattern2);
        eString = sentences[i].Replace(sPattern2, "");
    }
    else if (System.Text.RegularExpressions.Regex.IsMatch(sentences[i], sPattern3, System.Text.RegularExpressions.RegexOptions.IgnoreCase))
    {
        Console.WriteLine(" (match for '{0}' found)", sPattern3);
        eString = sentences[i].Replace(sPattern3, "");
    }
    else
    {
        Console.WriteLine();
        eString = sentences[i];
    }


    fString[i] = eString;
}


// ok, let's look at the end result
foreach(string s in fString)
{

    Console.WriteLine(s);
}

// stop the app from shutting down before seeing results
Console.ReadLine(); 
\$\endgroup\$
4
\$\begingroup\$

Well, the first thing I'd do is put the patterns in a list, and loop through matching them. The second thing would be general cleanup; use namespaces where you can, one-liner if/else clauses don't need braces, use var where the type is obvious, etc etc. This gets rid of that if-elseif structure and makes the code much more concise.

As far as merging the patterns, you could match on a combination of two:

  • "xmlns:(d5p1|d2p1)=\".*?\"" (we don't care what the exact namespace definition is, just don't match greedily)
  • "(d5p1|d2p1):nil=\"true\""

If either of these patterns match, replace the matching text with an empty string:

    List<string> patterns = new List<string>{
       "xmlns:(d5p1|d2p1)=\".*?\"",
       "(d5p1|d2p1):nil=\"true\"",
    };

    var output = new string[sentences.Count()];
    for(int i=0; i < sentences.Count(); i++)
    {
        Console.WriteLine("{0,24}", sentences[i]); // 24 chars, left-justified.
        var formattedSentence = sentences[i];                       

        foreach(var pattern in patterns)
        {
           var len = formattedSentence.Length;
           formattedSentence = Regex.Replace(formattedSentence, pattern, String.Empty,
                 RegexOptions.IgnoreCase)

           if(formattedSentence.Length != len)
              Console.WriteLine(" (match for '{0}' found)", pattern);                                  
        }

        //Finally, this may have left some unneeded whitespace which we can consolidate.
        formattedSentence = Regex.Replace(formattedSentence, "  +", " ");

        output[i] = formattedSentence;
    }


    // ok, let's look at the end result
    foreach(var sentence in output)
       Console.WriteLine(sentence); 

One last thing. Hungarian notation - putting some type-specific flag at the beginning of your variable names - is EVIL in virtually any language used in an IDE, including .NET languages. IntelliSense tells you all you need to know about an object's type, if it happens to not be obvious via context; you shouldn't put it in the variable name. If you do, and the type changes, the variable name is now inconsistent and every usage of it needs to change (a task made easier by refactoring assistants like ReSharper, but if you have that you have absolutely no need for Hungarian).

\$\endgroup\$
  • \$\begingroup\$ Brilliantly said Keith S., I'll incorporate your suggestions subito pronto!:-) \$\endgroup\$ – plditallo Jul 26 '12 at 21:15
  • 2
    \$\begingroup\$ Don't forget to upvote and accept if this helped! We work for rep around here. \$\endgroup\$ – KeithS Jul 27 '12 at 19:01
  • \$\begingroup\$ sorry Keith I tried, but I'm so wet behind the ears here, i didn't have enough privileges to do so!:-| \$\endgroup\$ – plditallo Jul 28 '12 at 15:18
  • \$\begingroup\$ Ok, Keith--I was able to upvote you today--as I finally have enough points! Thanks again. Your suggestions were very helpful. \$\endgroup\$ – plditallo Aug 1 '12 at 0:34

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.