tags:

views:

143

answers:

8

I need to do a 2 rule "replace" -- my rules are, replace all open parens, "(" with a hyphen "-" and strip out all closing parens ")".

So for example this:

"foobar(baz2)" would become

"foobar-baz2"

I currently do it like this -- but, my hunch regex would be cleaner.

myString.Replace("(", "-").Replace(")", "");
+7  A: 

I wouldn't go to RegEx for this - what you're doing is just right. It's clear and straightforward ... regular expressions are unlikely to make this any simpler or clearer. You would still need to make two calls to Replace because your substitutions are different for each case.

LBushkin
Problem doesn't merit moving to Regex's... Keep it simple...
LorenVS
Just 1 call is needed (see my example). Nonetheless regular replacement is best in this case.
Ahmad Mageed
+1  A: 

Nope. This is perfectly clean.

Point is, you'd have to have two regexes anyway, because your substitution strins are different.

Thomas
Technically in .NET you can specify a delegate that will evaluate to the replacement string, but now we're just making it more unclean
LorenVS
One regex would suffice.
Ahmad Mageed
Technically, yes. But like LorenVS already noted, it just makes the whole expression less clear.
Thomas
+1  A: 

I'd say use what you have - it's more-easily readable/maintainable. Regexes are super powerful but also sometimes super confusing. For something this simple, I'd say don't even use Regexes.

Jaxidian
+3  A: 

Jamie Zawinski suddenly comes to my mind:

Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems.

So I also think LBushkin is right in this case. Your solution works and is readable.

tobsen
+1  A: 

I'd think a regex is going to be kind of brittle for this kind of thing. If your version of .NET has extension methods and you'd like a cleaner syntax that scales you might introduce an extension method like this:

public static class StringExtensions
{
    public static string ReplaceMany(this string s, Dictionary<string, string> replacements)
    {
        var sb = new StringBuilder(s);
        foreach (var replacement in replacements)
        {
            sb = sb.Replace(replacement.Key, replacement.Value);
        }
        return sb.ToString();
    }
}

So now you build up your dictionary of replacements...

var replacements = new Dictionary<string, string> { {"(", "-"}, {")", ""} };

And call ReplaceMany:

var result = "foobar(baz2)".ReplaceMany(replacements); // result = foobar-baz2

If you really want to show your intent you can alias Dictionary<string,string> to StringReplacements:

//At the top
using StringReplacements = System.Collections.Generic.Dictionary<string,string>;

//In your function
var replacements = new StringReplacements() { {"(", "-"}, {")", ""} };
var result = "foobar(baz2)".ReplaceMany(replacements);

Might be overkill for only two replacements, but if you have many to make it'll be cleaner than .Replace().Replace().Replace().Replace()....

Jason Punyon
+1  A: 

You CAN use one regex to replace both those occurrences in one line, but it would be less 'forgiving' than two single rule string replacements.

Example:

The code that would be used to do what you want with regex would be:

Regex.Replace(myString, @"([^\(]*?)\(([^\)]*?)\)", "$1-$2");

This would work fine for EXACTLY the example that you provided. If there was the slightest change in where, and how many '(' and ')' characters there are, the regex would break. You could then fix that with more regex, but it would just get uglier and uglier from there.

Regex is an awesome choice, however, for applications that are more rigid.

Albert Bori
A: 

Regex is overkill for such a simple scenario. What you have is perfect. Although your question has already been answered, I wanted to post to demonstrate that one regex pattern is sufficient:

string input = "foobar(baz2)";
string pattern = "([()])";
string result = Regex.Replace(input, pattern, m => m.Value == "(" ? "-" : "");
Console.WriteLine(result);

The idea is to capture the parentheses in a group. I used [()] which is a character class that'll match what we're after. Notice that inside a character class they don't need to be escaped. Alternately the pattern could've been @"(\(|\))" in which case escaping is necessary.

Next, the Replace method uses a MatchEvaluator and we check whether the captured value is an opening ( or not. If it is, a - is returned. If not we know, based on our limited pattern, that it must be a closing ) and we return an empty string.

Ahmad Mageed
A: 

Here's a fun LINQ-based solution to the problem. It may not be the best option, but it's an interesting one anyways:

public string SearchAndReplace(string input)
{ 
   var openParen = '(';
   var closeParen = ')';
   var hyphen = '-';
   var newChars = input
        .Where(c => c != closeParen)
        .Select(c => c == openParen ? hyphen : c);
   return new string(newChars.ToArray());
}

2 interesting notes about this implementation:

  • It requires no complicated regex, so you get better performance and easier maintenance.
  • Unlike string.Replace implementations, this method allocates exactly 1 string.

Not bad!

Judah Himango