How can I improve this linq to xml?

Question

Here are the problems with the LINQ to XML below:

• Is there a way to return an enumeration of the attributes so I don't have to do the foreach loop?

• It should only return elements that have a cardnum between 13 and 16 digits, but it appears to be returning numbers longer than that? Why?

• Is long.TryParse the best way to test if the 16 digit number is in fact a number?

• Also, is it possible to return not only elements that have attributes with 16 digit numbers, but also elements with inner text such as <ccnum>1234567890123456</ccnum> and then parse every child node of the parent node of <ccnum>, so for example, the xml would look like this:

   <details>
   <ccnum>283838383838383838</ccnum>
   <cvv>399</cvv>
   <exp>0202</exp>
   <name>joe</name>
   </details>
  

Here is the code:

    long numeric;

    string xml = @"<Details>
    <CreditCard cardnum='1234888888823456'
    ccv='123' 
    exp='0212' 
    cardType='1' 
    name='joe' />
    <CreditCard cardnum='123488888882345633333'
    ccv='123' 
    exp='0212' 
    cardType='1' 
    name='joe' />
    </Details>";

    XElement element = XElement.Parse(xml);
    IEnumerable<XElement> elementsWithPossibleCCNumbers = 
        element.Descendants()
               .Where(d => d.Attributes()
                            .Where(a => a.Value.Length >= 13 && a.Value.Length <= 16)
                            .Where(a => long.TryParse(a.Value, out numeric))
                            .Count() == 1).Select(x=>x);


    foreach(var x in elementsWithPossibleCCNumbers)
    {
        foreach(var a in x.Attributes())
        {
        //Check if the value is a number
        if(long.TryParse(a.Value,out numeric))
        {
            //Check if value is the credit card
            if(a.Value.Length >= 13 && a.Value.Length <= 16)
                xml = xml.Replace(a.Value, string.Concat(new String('*',a.Value.Length - 4),a.Value.Substring(a.Value.Length - 4)));
            else //If value is not a credit card, replace it with ***
                xml = xml.Replace(a.Value, "***");
        }
      }
    }

OK, I got why it I thought it was returning the number longer than 16, it was because, the first 16 digits are the same as the first number and I am just replacing that part, so I guess that brings up the question of how to just update the correct attribute.

Is a solution to updating the whole number is to use a regex boundary?

Answer 1

• To avoid the foreach loop:

  var element = XElement.Parse(xml);
  var elementsWithPossibleCCNumbers =
      element.Descendants()
              .Where(d => d.Attributes()
                  .Where(a => a.Value.Length >= 13 && a.Value.Length <= 16)
                  .Count(a => long.TryParse(a.Value, out numeric)) == 1);
  
  elementsWithPossibleCCNumbers
      .SelectMany(e => e.Attributes())
      .Where(a => long.TryParse(a.Value, out numeric))
      .ToList()
      .ForEach(a => a.Value = a.Value.Replace(a.Value, MaskNumber(a.Value)));
  

and declare this method:

    static string MaskNumber(string numericValue)
    {
        if (numericValue.Length >= 13 && numericValue.Length <= 16)
            return new String('*', numericValue.Length - 4) + numericValue.Substring(numericValue.Length - 4);

        return "***";
    }

• It should only return elements that have a cardnum between 13 and 16 digits [...] - glad you sorted that out :-)

• I think long.TryParse is a good way to check if all characters are digits. Alternatively, you could use the regex that @Henk Holterman suggested in his answer - that also gets rid of the Length comparison, making the code shorter and more readable.

• In the case of elements with inner text, you should use element.Value instead of foreach(a in element.Attributes) -> a.Value

Answer 2

I would use something like:

    var rexCardnum = new Regex(@"^\d{13,16}$");
    var element = XElement.Parse(xml);
    var elementsWithPossibleCCNumbers =
        element.Descendants("CreditCard")
        .Where(d => rexCardnum.IsMatch(d.Attribute("cardnum").Value));

or, when cardnum could be missing:

     .Where(d => d.Attribute("cardnum") != null 
           && re.IsMatch(d.Attribute("cardnum").Value));