Refactoring with Lambda's and Delegates

Question

I've just installed VS2008 and have run into a problem that I'm sure can be solved with either lambda's or delegates (or a combination!).

    private string ReadData(TcpClient s, string terminator)
    {
        // Reads a byte steam into a string builder until either data is unavailable or the terminator has not been reached
        var sb = new StringBuilder();
        do
        {
            var numBytesRead = s.GetStream().Read(byteBuff, 0, byteBuff.Length);
            sb.AppendFormat("{0}", Encoding.ASCII.GetString(byteBuff, 0, numBytesRead));
        } while (s.GetStream().DataAvailable && !sb.ToString().Contains(terminator));

        return sb.ToString();
    }

The problem is, sometimes I need to check if the string contains either of two different values. Sometimes I may need to check it for three values.

So what I propose, is to change " !sb.ToString().Contains(terminator)" to a function that is passed into the method.

I could write my different functions such as:

private bool compare1(string s, string t) {
    return s.contains(t)
}

private bool compare2(string s, string t1, string t2) {
    return (s.compare(t1) or s.compare(t2)
}

// etc...

Then when I want to compare with 3 different values, create a delegate to one of these functions, then pass that to the ReadData() method.

I'm very clueless when it comes to delegates, and I'm not sure if this seems like the right place for a lambda but something is telling me it is.

The calling code is this:

            // Enter username .
        if (HasData(s,"login:"))
            SendData(s, switchUser + TelnetHelper.CRLF);

HasData is identical to ReadData, but returns a bool instead of a string (which I'd also like to factor out into one method using some trickery - but that's a secondary question - feel free to answer that though.

Just for reference:

     private bool HasData(TcpClient s, string terminator)
    {
        // Reads a byte steam into a string builder until either data is unavailable or the terminator has not been reached
        var sb = new StringBuilder();
        do
        {
            var numBytesRead = s.GetStream().Read(byteBuff, 0, byteBuff.Length);
            sb.AppendFormat("{0}", Encoding.ASCII.GetString(byteBuff, 0, numBytesRead));
        } while (s.GetStream().DataAvailable && !sb.ToString().Contains(terminator));

        return sb.ToString().Contains(terminator);
    }

Answer 1

It sounds like you're looking for a predicate function. Instead of hard coding the check, take a delegate as a parameter than can do the check

    private string ReadData(TcpClient s, Func<string,bool> predicate)
    {
        // Reads a byte steam into a string builder until either data is unavailable or the terminator has not been reached
        var sb = new StringBuilder();
        do
        {
            var numBytesRead = s.GetStream().Read(byteBuff, 0, byteBuff.Length);
            sb.AppendFormat("{0}", Encoding.ASCII.GetString(byteBuff, 0, numBytesRead));
        } while (s.GetStream().DataAvailable && !predicate(sb));

        return sb.ToString();
    }

Then you can create several wrappers which just create the appropriate delegate and pass it down

public bool HasData(TcpClient c, string terminator) {
  return HasData(c, (s) => s.Contains(terminator));
}

public bool HasData(TcpClient c, string t1, string t2) {
  return HasData(c, (s) => s.Contains(t1) || s.Contains(t2));
}

You can even build a delegate on the fly based on arbitrary number of terminators

public bool HasData(TcpClient c, params string[] terminatorList) {
  return HasData(c, (s) => terminatorList.Where(x => s.Contains(x)).Any());
}

Answer 2

One option would be to overload the ReadData() method to take a string array containing the values that you are checking for. Using an extension method, you could extend Contains() to take a string array.

Your ReadData() method could be:

private string ReadData(TcpClient s, string[] terminators) {
    // Reads a byte steam into a string builder until either data is unavailable or the terminator has not been reached
    var sb = new StringBuilder();
    do
    {
        var numBytesRead = s.GetStream().Read(byteBuff, 0, byteBuff.Length);
        sb.AppendFormat("{0}", Encoding.ASCII.GetString(byteBuff, 0, numBytesRead));
    } while (s.GetStream().DataAvailable && !sb.ToString().Contains(terminators));

    return sb.ToString();
}

The Contains() method extension could be:

public static bool Contains ( this String str , String[] testValues )
{
 foreach ( var value in testValues )
 {
  if ( str.Contains( value ) )
   return true;
 }
 return false;
}

This implementation eliminates the need to create a new predicate each time you have a different number of strings to test for.

Answer 3

Because the syntax of the lambdas is somewhat foreign to myself (and the rest of my team) I ended up going with a slightly different solution. I couldn't figure out the syntax of .All() when modified from the .Any() function above.

I needed an .All() function as well, to ensure all the terminators in the list were found. So I ended up going with something like the following:

delegate bool Predicate (string s, params [] string terminators);

bool HasAll(string s, params string [] terminators) {
    foreach (var t in terminators) {
       if (!s.contains(t)) return false;
    }
    return true;
}

bool HasAny(string s, params string [] terminators) {
    foreach (var t in terminators) {
        if (s.contains(t)) return true;
    }
    return false;
}
// Just looking now, I could also pass in a bool to switch between the two and remove one of these functions. But this is fairly clear


string ReadData(TcpClient sock, Function predicate, params [] string terminators) {
    var sb = new StringBuilder();
    do
    {  
        var numBytesRead = s.GetStream().Read(byteBuff, 0, byteBuff.Length);
        sb.AppendFormat("{0}", Encoding.ASCII.GetString(byteBuff, 0, numBytesRead));
    } while (s.GetStream().DataAvailable && !predicate(sb.ToString(), terminators);

    return sb.ToString();
}

Then the calling code looks like:

private void someFunc() 
{
    Predicate any = new Predicate(HasAny);
    Predicate all = new Predicate(HasAll);
    String response;

    // Check all strings exist
    response = ReadData(this.sock, all, "(", ")", "->")
    if (all(response, "(", ")", "->")
        SendData(this.sock, ...);

    // Check any string exists
    response = ReadData(this.sock, any, "Hi", "Hey", "Hello");
    if (any(response, "Hi", "Hey", "Hello"))
       SendData(this.sock, ...);
}

I'll probably add null checks into the Has[Any|All] functions, reverse the do..while to a while, and just check response != null instead of duplicating the params. This solutions suits all my use cases and is fairly human readable I think. As long as I make the small changes I mentioned just above.

This whole thing highlights for me my need to learn lambda expressions though!