4
\$\begingroup\$

My problem is that I have a sequence, potentially infinite, of strings. Let's suppose it to be

{"Mickey Mouse", "Bugs Bunny", "Winnie the Pooh"}

I have to make an exthension-method Initials that returns a sequence of their capital letters in uppercase, so in this case, doing: 

var names = new string [] {"Mickey  Mouse", "Bugs    Bunny", "Winnie  the Pooh"}
foreach (var cl in names . Initials ())
Console.WriteLine(cl);

returns, in order, MM, BB, WTP. Note that whitespaces have not to be considered.

After hours of staring at the screen, i managed to write this not so goodlooking code:

public static IEnumerable<string> Initials(this IEnumerable<string> names)
{
    var res = new List<string>();
    var  listOfArrayOfStrings = names.Select(s => s.Split());
    foreach (var arrayOfStrings in listOfArrayOfStrings)
    {
        string newString = "";
        foreach (var singleString in arrayOfStrings)
        {
            if (!string.IsNullOrWhiteSpace(singleString)) //no whitespaces
            {
                var temp =  char.ToUpper(singleString[0]).ToString();       //first upper String
                newString += temp;
            }
        }
        res.Add(newString);
    }
    return res;
}

Then, my disliked R# suggested me to do some refacory and then it became the following:

var  listOfArrayOfStrings = names.Select(s => s.Split());
            return listOfArrayOfStrings.Select(arrayOfStrings => 
                (from singleString in arrayOfStrings 
                 where !string.IsNullOrWhiteSpace(singleString) 
                 select char.ToUpper(singleString[0]).ToString())
                 .Aggregate("", (current, temp) => current + temp))
                 .ToList();

Now, I have two questions about this:

  1. Is this, in your opinion, a good way to reach my aim?

  2. I will have to do a written test where I'll have to do something like that. How can I "guess" something like that without R#? I mean, I'm still having difficulties understanding all that Linq stuff. Have you got any idea to how I can approach problems like this? Often these test are about managing IEnumerables, so Linq could be very helpful, but I rarely understand the right sequence of Linq queries.

\$\endgroup\$

2 Answers 2

Reset to default
4
\$\begingroup\$

Start by writing a method that returns the initials of a single string.

private static string GetInitials(string name)
{
    ???
}

You have the right idea, but there are some overloads we can use that will make the code a bit nicer:

private static string GetInitials(string name)
{
    var initials = name
        .Split((char[])null, StringSplitOptions.RemoveEmptyEntries)
        .Select(w => char.ToUpper(w[0]));
    return new string(initials.ToArray());
}

Here we've used the overload of Split that won't return empty entries, so we don't have to check for empty entries. We've also used the string(char[]) constructor instead of building up the string ourselves.

Now Initials we be written as

return names.Select(GetInitials);

There's a problem with both versions in that the method will not return if the input sequence is infinite (which you've mentioned is a case you want to handle).

Suppose we have this helper method to create an infinite sequence of a single value

private static IEnumerable<T> Repeat<T>(T value)
{
    while (true)
    {
        yield return value;
    }
}

Then our method should be able to handle this client code:

foreach (var cl in Repeat("Mickey Mouse").Initials().Take(5))
{
    Console.WriteLine(cl);
}

The versions posted must process every element of the sequence before they return, which isn't possible for infinite sequences.

\$\endgroup\$
1
  • \$\begingroup\$ Note that there's a bit of magic going on with names.Select(GetInitials) - the compiler is inferring a number of things. This syntax is called a "method group", and is equivalent to names.Select(x => GetInitials(x)). ReSharper makes a suggestion here, to "convert to method group", because the parameter-passing can be inferred since the method only takes a single parameter, x. Don't blindly apply R# suggestions - use R# suggestions to learn when and why such or such formulation is applicable, and only apply suggestions you're comfortable with and that improve readability. \$\endgroup\$ Commented Jan 19, 2015 at 0:19
2
\$\begingroup\$ 
var names = new string [] {"Mickey  Mouse", "Bugs    Bunny", "Winnie  the Pooh"}
foreach (var cl in names . Initials ())
Console.WriteLine(cl);

Wow this is an utterly confusing way of writing code. Compare to the equivalent:

var names = new string [] {"Mickey  Mouse", "Bugs    Bunny", "Winnie  the Pooh"}
foreach (var cl in names.Initials())
{
    Console.WriteLine(cl);
}

What are the differences?

  • Proper scope-delimiting braces - a foreach block defines a scope; using curly braces makes that a lot more apparent.
  • Removed extraneous whitespace - dot-notation (object.Method()) is a LOT easier to read when there's no whitespace before/after the dot.

Your specifications aren't clear at all:

return a sequence of their capital letters in uppercase

Capital letters are usually, well, uppercase. And "Winnie the Pooh" doesn't have a capital "T" but your method returns "WTP" for it. I think you have simply misphrased what the function does; if the Initials method had XML comments, the description you gave would be completely misleading, assuming the function does what it's supposed to be doing.

var  listOfArrayOfStrings = names.Select(s => s.Split());

Consider naming variables after their meaning rather than their type - here you made a relatively confusing mistake: the result of a Select call can never be a List, so that list of arrays of strings is actually not a list.

ReSharper suggestions aren't recommendations - they're suggestions, and that doesn't mean you have to implement them... especially when readability is taking a beating, as is the case here.

\$\endgroup\$
3
  • \$\begingroup\$ Some clarifications: Regarding the whitespace-dot-notation it has been a mistake due to tiredness because i took the text from a pdf and pasting it on here i didn't remember to fix the whitespaces. When there's a foreach, if ecc and it concerns only one line of code, i usually remove the braces because teacher always tell me to remove "useless" braces. The method has to make the first letter of each word uppercase, even if it isn't a proper name. I used that names but i could have used "lemon tree", " broken wings" and other stuff and what i aspect this method to return is "LT","BW"ecc. \$\endgroup\$ Commented Jan 19, 2015 at 8:18
  • 1
    \$\begingroup\$ @Sanci If you remember Apple's 'goto fail' fiasco last year (and if you don't, do a quick search), use that as an example to remind your teacher of why braces are not 'useless' in this case. Readability is VERY important in production code! \$\endgroup\$ Commented Jan 19, 2015 at 11:49
  • 1
    \$\begingroup\$ @Sanci even without the braces, the line belongs in another scope and should at least be indented to reflect that. Having it unbraced and at the same level of indentation as everything else is asking for maintenance bugs to happen. And I don't agree with your teacher, braces are only useless when they define a scope that doesn't need to exist. When the scope is dictated by language keywords, braces make everything clear and unambiguous. \$\endgroup\$ Commented Jan 19, 2015 at 12:00

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.