CODEWITHSUNDEEP
c#
Nick Spicer
Nick Spicer

Reputation: 2707

String.Replace on null values

I have a Replace method which can accept Xml node names as variables, the Replace will be evaluated on each row in the Xml getting the value of the node before it evaluates the expression. Therefore if some nodes don't exist the Replace will be evaluating on a null.

Given this condition is the following code is justified?

<chocolates>
    <chocolate>
        <name>Mars Bar</name>
    </chocolate>
    <chocolate>
        <name>Bounty</name>
        <type>Coconut</type>
    </chocolate>
</chocolates>

Replace(type, 'C', 'c')

public string Replace(string a, string b, string c) 
{
    if (a == null) return null;
    if (b == null || c == null) return a;

    return a.Replace(b, c); 
}

Upvotes: 1

Views: 17035

Answers (4)

Dmitrii Bychenko
Dmitrii Bychenko

Reputation: 186668

The code can be simplified:

// When designing public methods you'd rather give to the parameters more
// descriptive names than "a", "b", "c", e.g.
// "source", "toFind", "toReplace"
public string Replace(string a, string b, string c) 
{
    if ((a == null) || (b == null) || (c == null))
      return a;

    return a.Replace(b, c); 
}

Or even further with a help of the trenary operator:

public string Replace(string a, string b, string c) 
{
    return ((a == null) || (b == null) || (c == null)) ? a : a.Replace(b, c);
}

You should use String, e.g. "c", "C" not Char 'C', 'c' on call:

// All three arguments should be strings
String type = ...
...
Replace(type, "C", "c");

Upvotes: 2

Mikey Mouse
Mikey Mouse

Reputation: 3088

An extension method would look neater I think

    public static class MyTestClass
    {
      public static string MyReplace(this string stringToSearch, string find, string replaceWith)
      {
        if(stringToSearch == null) return null;
        if(string.IsNullOrEmpty(find) || replaceWith == null) return stringToSearch;
        return stringToSearch.Replace(find, replaceWith);
      }
    }

Then you could just call

      type.MyReplace('C','c');

Edit: Added the full code for an extension class, and added isNullOrEmpty instead of just a null check for "find" as you don't wanna pass an empty string to the Replace call

Upvotes: 0

Jaroslav Kubacek
Jaroslav Kubacek

Reputation: 1447

You need also to check that b is not empty. Otherwise argument exception will occure 

Console.WriteLine(Replace("dsfa", "", ""));
// Define other methods and classes here
public string Replace(string a, string b, string c) 
{
    if (a == null) return null; 
    if (String.IsNullOrEmpty(b) || c == null) return a;

    return a.Replace(b, c); 
}

Upvotes: 0

Flo
Flo

Reputation: 986

Maybe you should call your function by :

Replace(type,"C","C");

(ofcourse "type" also needs to be a string value)

or try this to compare string to null :

if(a.Equals(null)) return null;

Upvotes: 0

Related Questions