How correctly replace string

Question

In my code I find all matches elements and replace it with special values.

Regex imgRule = new Regex("img id=\\\".+?\\\"");
                    MatchCollection matches = imgRule.Matches(content.Value);
                    string result = null;
                    foreach (Match match in matches)
                        result = match.Value;

                    if (result != null)
                    {
                        var firstOrDefault = node.ListImages.FirstOrDefault();
                        if (firstOrDefault != null)
                        {
                            var htmlWithImages = content.Value.Replace(result, string.Format("img src='{0}' class='newsimage' width='300'", firstOrDefault.ImageUrlId));
                            node.Content = htmlWithImages;
                        }
                    }

But, my code is wrong because if there is more than one match it replace only the last one, how can I correct my code for replace all matches in text?

What is the type of content? Where is it declared? What is the type of node? Where is it declared? — Mark Byers
– Mark Byers, Commented May 28, 2012 at 14:16

Mark Byers · Accepted Answer · 2012-05-28 14:01:08Z

3

You are missing curly braces around the body of your for loop. Without the curly braces the only line that gets executed multiple times is the first one.

Try this instead:

foreach (Match match in matches)
{                                    // added curly brace here
    result = match.Value;

    if (result != null)
    {
        var firstOrDefault = node.ListImages.FirstOrDefault();
        if (firstOrDefault != null)
        {
            var htmlWithImages = content.Value.Replace(result,
                string.Format("img src='{0}' class='newsimage' width='300'",
                              firstOrDefault.ImageUrlId));
            node.Content = htmlWithImages;
        }
    }
}                                    // added curly brace here

I would also like to add two further points:

There is a method called Regex.Replace that you can use instead of first finding the strings you want to replace using a regex, and then using string.Replace.
If you are trying to parse HTML it would be better to use a HTML parser. Take a look at HTML Agility Pack to see if it could be an easier way to solve your problem.

edited May 28, 2012 at 14:01

answered May 28, 2012 at 13:51

Mark Byers

844k202 gold badges1.6k silver badges1.5k bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

revolutionkpi Over a year ago

thanks, in any case the main question is: at first time I replace content.Value with first match and then I replace content.Value fith second match, and the first replace is not save

Mark Byers Over a year ago

@revolutionkpi: One strange thing about your code is that you are reading from content.Value and assigning to node.Content. Are you sure that when you assign to node.Content that the changes are visible in content.Value? I think it would make more sense to read the content out of the document and into a plain string, make the desired replacements on the string (s = s.Replace(...);), then reassign the final result back to the document at the end (not inside the loop). But you really should be using HTML Agility Pack for this sort of thing. It will make your life much easier.

Mark Byers Over a year ago

@revolutionkpi: By the way, my answer was actually to your "main" question. If you think my answer was not answering your question then I think you may have either misread or else misunderstood my answer. It's also possible that you have not one, but multiple errors in your code. Then I suggest you fix them one by one, and don't be discouraged that fixing one error doesn't immediately solve all your problems. Sometimes you need to take many small steps to reach your final destination.

Ras · Accepted Answer · 2012-05-28 13:51:40Z

1

foreach (Match match in matches)
{
    result = match.Value;

    if (result != null)
    {
        var firstOrDefault = node.ListImages.FirstOrDefault();
        if (firstOrDefault != null)
        {
            var htmlWithImages = content.Value.Replace(result, string.Format("img src='{0}' class='newsimage' width='300'", firstOrDefault.ImageUrlId));
            node.Content = htmlWithImages;
        }
    }
}

answered May 28, 2012 at 13:51

Ras

6281 gold badge11 silver badges29 bronze badges

Comments

Rohan Büchner · Accepted Answer · 2012-05-28 13:59:25Z

1

I think you might be missing a set of brackets around your loop...

Only this line gets looped. That's why your code only updates the last entry, as result is set to the last item in the collection (on the last iteration of the foreach)

         foreach (Match match in matches) 
                      result = match.Value;

Corrected code

  Regex imgRule = new Regex("img id=\\\".+?\\\"");
                        MatchCollection matches = imgRule.Matches(content.Value);
                        string result = null;
                        foreach (Match match in matches) {
                            result = match.Value;

                           if (result != null)
                           {
                               var firstOrDefault = node.ListImages.FirstOrDefault();
                               if (firstOrDefault != null)
                               {
                                   var htmlWithImages = content.Value.Replace(result,    string.Format("img src='{0}' class='newsimage' width='300'", firstOrDefault.ImageUrlId));
                                   node.Content = htmlWithImages;
                               }
                           }   
                        }

edited May 28, 2012 at 13:59

answered May 28, 2012 at 13:51

Rohan Büchner

5,4035 gold badges66 silver badges108 bronze badges

1 Comment

revolutionkpi Over a year ago

thanks, but when I replace string with the second match the first is no save, so in result there is only the last replacement

AwesomeTown · Accepted Answer · 2012-05-28 13:52:19Z

0

Would the Regex.Replace method not simplify what you're trying to accomplish?

http://msdn.microsoft.com/en-us/library/system.text.regularexpressions.regex.replace(v=vs.71).aspx

answered May 28, 2012 at 13:52

AwesomeTown

2,9103 gold badges29 silver badges42 bronze badges

Collectives™ on Stack Overflow

How correctly replace string

4 Answers 4

3 Comments

Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

3 Comments

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related