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.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jan 18, 2015 at 23:10
\$\endgroup\$

2 Answers 2

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.

answered Jan 18, 2015 at 23:35
\$\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.

answered Jan 19, 2015 at 0:21
\$\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

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.