Comma Quibbling
[UPDATE: Holy goodness. Apparently this was a more popular pasttime than I anticipated. There's like a hundred solutions in there. Who knew there were that many ways to stick commas in a string? It will take me some time to go through them all, so don't be surprised if it's a couple of weeks until I get them all sorted out.]
The point of Monday’s post about comma-separated lists was not so much about the actual problem; it’s a rather trivial problem. Rather, I wanted to make two points. First, stating the actual problem rather than a much harder and more general version of the problem is likely to get you a realistic solution to your actual problem much faster. And second, reworking the statement of the problem into an equivalent but structurally different statement is a great way to see solutions that you might have otherwise missed.
But whenever I make a post illustrating such points with a specific example, lots of people pipe up with their ideas for how to solve the specific example. Which is awesome; I encourage this behaviour.
So in that spirit, here’s a slightly harder version of the string concatenation problem, just for the fun of it. Write me a function that takes a non-null IEnumerable<string> and returns a string with the following characteristics:
(1) If the sequence is empty then the resulting string is "{}".
(2) If the sequence is a single item "ABC" then the resulting string is "{ABC}".
(3) If the sequence is the two item sequence "ABC", "DEF" then the resulting string is "{ABC and DEF}".
(4) If the sequence has more than two items, say, "ABC", "DEF", "G", "H" then the resulting string is "{ABC, DEF, G and H}". (Note: no Oxford comma!)
I think you get the idea. You can post your solution in the comments or use the link on the blog page to email your solution to me.
The strings in the sequence can be assumed to be non-null but can otherwise be any string value, including empty strings or strings containing commas, braces and "and".
There’s no size limit on the sequence; it could be tiny, it could be thousands of strings. But it will be finite.
All you get are the methods of IEnumerable<string>; if you want to make that thing into a list or an array, you’re going to need to do that explicitly rather than casting it and hoping for the best.
I am particularly interested in solutions which make the semantics of the code very clear to the code maintainer.
Of course, C# is most interesting to me, but if there are neat ways to express this in other languages, I’d love to see them too.
If there are any particularly amusing or interesting implementations I’ll dissect them on the blog in a future episode, probably in a week or so. I’m not going to have time to do a detailed analysis of every one.
And… go!
Comments
Anonymous
April 15, 2009
What should be returned for new[] { "", ",", "}" }? "{, , and }}"? "{"", "," and "}"}"? Something else? The former seems reasonable. Basically, I don't expect you to parse the inputs. -- EricAnonymous
April 15, 2009
This is what I came up with. private string buildCommaSeperatedList(IEnumerable<string> Items)
{
//Create List from IEnumerable
List<string> ListItems = new List<string>();
foreach (string item in Items)
{
ListItems.Add(item);
}
//Instantiate Return Value
StringBuilder CommaSeperatedList = new StringBuilder();
//Add Ending Bracket
CommaSeperatedList.Append("}");
//Get the index of the last item in the list
int LastIndex = ListItems.Count - 1;
//Loop through the list in reverse order.
for (int i = LastIndex; i >= 0; i--)
{
//Add " and " between the last two items.
if (i == LastIndex - 1) CommaSeperatedList.Insert(0, " and ");
//Add commas for all items before the second to last item.
if (i < LastIndex - 1) CommaSeperatedList.Insert(0, ", ");
//Add List Item
CommaSeperatedList.Insert(0, ListItems[i]);
} //Add beginning bracket
CommaSeperatedList.Insert(0, "{");
//Return Comma Seperated List as a string
return CommaSeperatedList.ToString();
} Care to take a stab at what the asymptotic order of this algorithm is? -- EricAnonymous
April 15, 2009
Great question. Hopefully this will format reasonably... if not I'll mail it to Eric. The restatement of the problem is in the XML comments for the method. using System;
using System.Collections.Generic;
using System.Text;
class CommaTeaser
{
static void Main()
{
Test();
Test("ABC");
Test("ABC", "DEF");
Test("ABC", "DEF", "G", "H");
}
static void Test(params string[] words)
{
Console.WriteLine(JoinWords(words));
}
/// <summary>
/// Joins words as per Eric's post.
/// </summary>
/// <remarks>
/// Restating the problem:
/// 1) We always start with "{" and end with "}"
/// 2) The last word has no suffix
/// 3) The penultimate word has a suffix of " and "
/// 4) All other words have a suffix of ", "
/// Now to work out the last and penultimate words, we just
/// have to keep a "buffer" of the last two words we've
/// seen.
/// </remarks> static string JoinWords(IEnumerable<string> words)
{
StringBuilder builder = new StringBuilder("{");
string last = null;
string penultimate = null;
foreach (string word in words)
{
// Shuffle existing words down
if (penultimate != null)
{
builder.Append(penultimate);
builder.Append(", ");
}
penultimate = last;
last = word;
}
if (penultimate != null)
{
builder.Append(penultimate);
builder.Append(" and ");
}
if (last != null)
{
builder.Append(last);
}
builder.Append("}");
return builder.ToString();
}
}Anonymous
April 15, 2009
This is a little less dumb. private string buildCommaSeperatedList(IEnumerable<string> Items)
{
//Create List from IEnumerable
List<string> ListItems = new List<string>();
foreach (string item in Items)
{
ListItems.Add(item);
}
//Instantiate Return Value
StringBuilder CommaSeperatedList = new StringBuilder();
CommaSeperatedList.Append("{");//Add Ending Bracket
//Loop through the list.
for (int i = 0; i <= ListItems.Count - 1; i++)
{
//Add List Item
CommaSeperatedList.Append(ListItems[i]);
//Add " and " between the last two items.
if (i == ListItems.Count - 2) CommaSeperatedList.Append(" and ");
//Add commas for all items before the second to last item.
if (i < ListItems.Count - 2) CommaSeperatedList.Append(", ");
}
CommaSeperatedList.Append("}");//Add ending bracket
//Return Comma Seperated List as a string
return CommaSeperatedList.ToString();
}Anonymous
April 15, 2009
Here's mine that does a single pass without foreach. It's a little like Jon's, rolled up public string CommaList(IEnumerable<string> e)
{
using (var en = e.GetEnumerator())
{
// Handle 0 or 1 words
var word = en.MoveNext() ? en.Current : "";
if (!en.MoveNext())
return "{" + word + "}";
// Handle 2 or more words
var sb = new StringBuilder(word);
do
{
word = en.Current;
if (!en.MoveNext()) // This was the last word
return "{" + sb.ToString() + " and " + word + "}";
sb.Append(", ");
sb.Append(word);
} while (true);
}
}Anonymous
April 15, 2009
err, I mean, it's a little like Jon's rolled up, but my two buckets are "word" and "en.Current"Anonymous
April 15, 2009
The comment has been removedAnonymous
April 15, 2009
You can use the StringBuilder trick you described in the last post. Build the list as before, track the last comma and replace it with " and" Something along these lines (haven't tried to compile): string Join(IEnumerable<string> data) {
StringBuilder sb = new StringBuilder("{");
bool isFirst = true;
int off;
foreach(string s in data) {
off = sb.Length;
if(!isFirst) {
sb.Append(", ");
} else {
isFirst = false;
}
sb.Append(s);
}
if(off > 0) {
sb.Remove(off, 1);
sb.Insert(" and");
}
sb.Append("}");
}Anonymous
April 15, 2009
public static string CommaQuibbling(this IEnumerable<string> items) {
using (IEnumerator<string> enumtor = items.GetEnumerator())
return (new StringBuilder()).AppendBracketed(enumtor).ToString();
} static StringBuilder AppendBracketed(this StringBuilder builder, Enumerator<string> enumtor) {
return builder.Append('{').AppendNoneOrMore(enumtor).Append('}');
} static StringBuilder AppendNoneOrMore(this StringBuilder builder, IEnumerator<string> enumtor) {
return enumtor.MoveNext()
? builder.AppendOneOrMore(enumtor.Current, enumtor)
: builder;
} static StringBuilder AppendOneOrMore(this StringBuilder builder, string first, IEnumerator<string> enumtor) {
return enumtor.MoveNext()
? builder.AppendTwoOrMore(first, enumtor.Current, enumtor)
: builder.Append(first);
} static StringBuilder AppendTwoOrMore(this StringBuilder builder, string first, string second, IEnumerator<string> enumtor) {
return enumtor.MoveNext()
? builder.Append(first).Append(", ").AppendTwoOrMore(second, enumtor.Current, enumtor)
: builder.Append(first).Append(" and ").Append(second);
} Slick. But at what size list does this cause your program to crash with an out-of-stack exception? -- EricAnonymous
April 15, 2009
The comment has been removedAnonymous
April 15, 2009
This would be my version of a function that fulfills the requirement efficiently while being pretty clear in how it works class Program
{
static void Main(string[] args)
{
Console.WriteLine(BuildStringWithEnglishListSyntax(new string[] { }));
Console.WriteLine(BuildStringWithEnglishListSyntax(new string[] { "ABC" }));
Console.WriteLine(BuildStringWithEnglishListSyntax(new string[] { "ABC", "DEF" }));
Console.WriteLine(BuildStringWithEnglishListSyntax(new string[] { "ABC", "DEF", "G", "H" }));
Console.WriteLine(BuildStringWithEnglishListSyntax(new string[] { "", ",", "}" }));
Console.ReadKey(true);
}
const string EnglishListPrefix = "{";
const string EnglishListSuffix = "}";
const string IntermediateSeparator = ", ";
const string LastSeparator = " and ";
static string BuildStringWithEnglishListSyntax(IEnumerable<string> itemsEnumerable)
{
StringBuilder result = new StringBuilder(EnglishListPrefix);
using(IEnumerator<string> items = itemsEnumerable.GetEnumerator())
{
if(items.MoveNext()) // make sure it's not an empty list
{
bool isFirstString = true;
bool isLastString = false;
while(!isLastString)
{
string current = items.Current;
isLastString = !items.MoveNext();
if(!isFirstString)
{
result.Append(isLastString ? LastSeparator : IntermediateSeparator);
}
else
{
isFirstString = false;
}
result.Append(current);
}
}
}
result.Append(EnglishListSuffix);
return result.ToString();
}
}Anonymous
April 15, 2009
> I am particularly interested in solutions which make the semantics of the code very clear to the code maintainer. Readability is terseness. static string JoinStrings(IEnumerable<string> strings) { var list = strings.ToList(); if (list.Count == 0) return "{}"; if (list.Count == 1) return "{" + list.First() + "}"; return "{" + string.Join(", ", list.GetRange(0, list.Count - 1).ToArray()) + " and " + list.Last() + "}"; }Anonymous
April 15, 2009
I didn't include comments, but this works with lazy evaluation of the enumerable. public static string FormatString(IEnumerable<string> stringsToFormat) { const string Separator = ", "; StringBuilder builder = new StringBuilder(); builder.Append('{'); string lastItem = string.Empty; foreach (string item in stringsToFormat) { builder.Append(item + Separator); lastItem = item; } int lengthOfReplacedString = (lastItem.Length + (Separator.Length * 2)); if (lengthOfReplacedString < builder.Length) { builder.Replace( Separator + lastItem + Separator, " and " + lastItem, builder.Length - lengthOfReplacedString, lengthOfReplacedString); } else { builder.Replace(Separator, string.Empty); } builder.Append('}'); return builder.ToString(); }Anonymous
April 15, 2009
The first reasonable idea I came up with was to create a list of KeyValuePairs, where the Value is the separator that goes before the value. I note that you have redefined the meaning of an existing class rather than defining your own. I personally would use KeyValuePair only for storing pairs of keys and their related values, so that the code is clear to the reader. If you want an "item and separator pair" class, I'd either use a general-purpose tuple, or define an "ItemAndSeparatorPair" class. That said, nice solution. -- Eric public static string ConcatenateSequence(IEnumerable<string> stringSequence)
{
var strings = stringSequence.ToList().ConvertAll(input => new KeyValuePair<string, string>(input, ", "));
if (strings.Count > 1)
{
var lastItem = strings[strings.Count - 1];
strings[strings.Count - 1] = new KeyValuePair<string, string> lastItem.Key, " and ");
}
var sequenceBuilder = new StringBuilder("{");
bool isFirst = true;
foreach (KeyValuePair<string, string> valueSeparatorPair in strings)
{
if (!isFirst)
{
sequenceBuilder.Append(valueSeparatorPair.Value);
}
sequenceBuilder.Append(valueSeparatorPair.Key);
isFirst = false;
}
sequenceBuilder.Append("}");
return sequenceBuilder.ToString();
}Anonymous
April 15, 2009
I agree with Olivier that readability is terseness. That solution is clear and takes very little code. More lines equals more bugs.Anonymous
April 15, 2009
Here are a few variations on a theme: string[] strings = { "ABC", "DEF", "G", "H" };
IEnumerable<string> separators = new[] { "", " and " }
.Concat(Enumerable.Repeat(", ", int.MaxValue));
string result = "{" + strings
.Reverse()
.Select((str, index) => str + separators.ElementAt(index))
.Aggregate((s1, s2) => s2 + s1) + "}";
//---------
int position = 0;
StringBuilder result2 = strings.Reverse().Aggregate(new StringBuilder("{}"),
(sb, str) => sb.Insert(1, position++ == 1 ? " and " : position > 1 ? ", " : "").Insert(1, str));Anonymous
April 15, 2009
I went for clarity over efficiency. public static string Format(IEnumerable<String> words)
{
int count = words.Count();
if (count == 0) return "{}";
if (count == 1) return string.Format("{{{0}}}", words.Single());
string commaDelimited = words.Take(count - 1).Aggregate((list, word) => string.Format("{0}, {1}", list, word));
return string.Format("{{{0} and {1}}}", commaDelimited, words.Last());
}Anonymous
April 15, 2009
The comment has been removedAnonymous
April 15, 2009
static string GetCombined(IEnumerable<string> strings)
{
string opening = "{";
var builder = new StringBuilder(opening);
var enumerator = strings.GetEnumerator();
bool hasNext = enumerator.MoveNext();
while (hasNext)
{
string s = enumerator.Current;
hasNext = enumerator.MoveNext();
if (builder.Length > opening.Length) // after the opening curly brace
builder.Append(hasNext ? ", " : " and ");
builder.Append(s);
}
builder.Append('}');
return builder.ToString();
}Anonymous
April 15, 2009
>> Extension methods << public static void IterateIndex<T>(this IEnumerable<T> items, Action<int, T> action) { IterateIndex(items, action, 0); } public static void IterateIndex<T>(this IEnumerable<T> items, Action<int, T> action, int idx) { if (items == null) throw new ArgumentNullException("items"); if (action == null) throw new ArgumentNullException("action"); IEnumerator<T> enumerator = items.GetEnumerator(); //Adjusting values for (int count = 0; count < idx; count++) enumerator.MoveNext(); while (enumerator.MoveNext()) { action(idx, enumerator.Current); idx++; } } >> Function << static string GetComma(IEnumerable<string> strings) { StringBuilder sb = new StringBuilder(); sb.Append("{"); var count = strings.Count(); strings.IterateIndex<string>((i, s) => { if (count > 1 && i == count - 1) { sb.Append(" AND "); } else if (i > 0) { sb.Append(","); sb.Append(" "); } sb.Append(s); }); sb.Append("}"); return sb.ToString(); } >>Execute<< var s1 = new List<string>(); var s2 = new List<string>() { "One" }; var s3 = new List<string>() { "One", "Two" }; var s4 = new List<string>() { "One", "Two", "Three", "Four" }; Console.WriteLine(GetComma(s1)); Console.WriteLine(GetComma(s2)); Console.WriteLine(GetComma(s3)); Console.WriteLine(GetComma(s4));Anonymous
April 15, 2009
Here's mine. It's not particularly brilliant or elegant, but at least it's short and easy to understand ;) The main drawback of this solution is that the strings are actually enumerated twice (in ToArray then in Join), although there's no explicit loop private static string FormatList(IEnumerable<string> list) { string[] tab = list.ToArray(); int n = tab.Length; string tmp; if (n > 1) { tmp = String.Join(", ", tab, 0, n - 1); tmp += " and " + tab[n - 1]; } else { tmp = String.Join(" and ", tab); } return "{" + tmp + "}"; }Anonymous
April 15, 2009
Oh great, Jon Skeet posted! Of course I'm just kidding. I'm sure some of my ideas have already been posted but that's ok... Here's the meat and potatoes: public static string FormatAsString(this IEnumerable<string> Sequence, string ItemSeparator, string LastItemSeparator)
{
var formattedString = new StringBuilder();
string prev = null;
foreach (string s in Sequence)
{
// the trick is to save the last item in the sequence for use outside of this loop
if (prev != null)
{
formattedString.Append(prev);
formattedString.Append(ItemSeparator);
}
prev = s;
}
if (prev != null)
{
if (formattedString.Length > 0)
{
formattedString.Append(LastItemSeparator);
}
formattedString.Append(prev);
}
formattedString.Append("}");
return "{" + formattedString.ToString();
} Which can be called like this: string GetSequenceAsFormattedString(IEnumerable<string> Sequence)
{
if (Sequence == null)
{
return string.Empty;
}
return Sequence.FormatAsString(", ", "and ");
}Anonymous
April 15, 2009
static string CommaSeparatedString(IEnumerable<string> input)
{
string first = "", last = null;
var rest = new StringBuilder();
IEnumerator<string> en = input.GetEnumerator();
if (en.MoveNext())
first = en.Current;
if (en.MoveNext())
last = en.Current;
while (en.MoveNext())
{
rest.Append(", ").Append(last);
last = en.Current;
}
if (last != null)
rest.Append(" and ").Append(last);
return "{" + first + rest + "}";
}Anonymous
April 15, 2009
I don't particularly like this solution from the perspective of the maintainer, but off the top of my head it's hard to find a much more readable solution. I look forward to seeing Eric's suggestion for how to do this. [STAThread] static void Main() { Console.WriteLine(GenerateSet(new string[] { })); Console.WriteLine(GenerateSet(new string[] { "ABC" })); Console.WriteLine(GenerateSet(new string[] { "ABC", "DEF" })); Console.WriteLine(GenerateSet(new string[] { "ABC", "DEF", "G", "H" })); Console.WriteLine(GenerateSet(new string[] { "Sample and other stuff", "Hello, World!", "XYZ", "42", "", "This is a test", "Last" })); } static private string GenerateSet(IEnumerable<string> items) { int itemsAdded = 0; StringBuilder sb = new StringBuilder(); sb.Append("{"); string current = null; foreach(string item in items) { if(current != null) { if(itemsAdded != 0) { sb.Append(", "); } sb.Append(current); ++itemsAdded; } current = item; } if(current != null) { if(itemsAdded != 0) { sb.Append(" and "); } sb.Append(current); } sb.Append("}"); return sb.ToString(); }Anonymous
April 15, 2009
Here's my solution, that I tried to keep as short as possible. I only have access to .NET 2.0 here so nothing fancy. You'll notice that this is pretty similar to Olivier's solution up above. I agree entirely that readability and terseness go hand in hand. I didn't want to mess with flags or anything else that was not directly related to the problem I was trying to solve. static string FancyConcat(IEnumerable<string> strEnumerable) { List<string> strList = new List<string>(strEnumerable); StringBuilder sb = new StringBuilder("{"); if (strList.Count > 0) { sb.Append(string.Join(", ", strList.GetRange(0, strList.Count - 1).ToArray())); if (strList.Count > 1) sb.Append(" and "); sb.Append(strList[strList.Count - 1]); } sb.Append("}"); return sb.ToString(); }Anonymous
April 15, 2009
static string Quibble(IEnumerable<string> strings) { string toReturn = "{"; int count = strings.Count(str => str != null); for (int i = 0; i < count; i++) { if (i == count-1) { toReturn += strings.ElementAt(i); } else { string delim = ", "; if (i == (count-2)) { delim = " and "; } toReturn += (strings.ElementAt(i) + delim); } } toReturn += "}"; return toReturn; }Anonymous
April 15, 2009
Here's my .02 static void Main(string[] args) { Console.WriteLine(list(null)); Console.WriteLine(list(new[] { "ABC" })); Console.WriteLine(list(new[] { "ABC", "DEF" })); Console.WriteLine(list(new[] { "ABC", "DEF", "G", "H" })); Console.WriteLine("Press any key"); Console.ReadLine(); } public static string list(IEnumerable<string> words) { string escape = "n"; string delim = ", "; string finalDelim = " and "; if (words == null) return "{}"; string sentence = "{" + string.Join(escape, words.ToArray()) + "}"; int lastCommaPosition = sentence.LastIndexOf(escape); if (lastCommaPosition > -1 && (sentence.IndexOf(escape) < lastCommaPosition)) { string remainder = sentence.Substring(lastCommaPosition + 1, sentence.Length - (lastCommaPosition + 1)); sentence = sentence.Replace(sentence.Substring(lastCommaPosition, sentence.Length - lastCommaPosition), finalDelim + remainder); } sentence = sentence.Replace(escape, delim); return sentence; }Anonymous
April 15, 2009
The comment has been removedAnonymous
April 15, 2009
I was going post my solution, but I realized mdefalco pretty much did it already. My only correction is that I would do a var arr = words.ToArray() and work off arr instead of words. The reason is that many IQueryable<string> come from LINQ2SQL data sources and can only be enumerated once, which has bit me in the rear too many times. As such, the ToArray would allow exactly one enumeration over the IEnumerable. ToList() would also do the job.Anonymous
April 15, 2009
public static IEnumerable<string> GetStrings()
{
yield return "ABC";
yield return "CDE";
yield return "G";
yield return "H";
} Here is the code to generate the output... var x = GetStrings().Reverse().Skip(1).Reverse().ToArray();
var z = String.Format("{0} {1} and {2} {3}", "{", String.Join(", ", x),
GetStrings().Last(), "}");
Console.WriteLine(z); What if the sequence only has one item? -- EricAnonymous
April 15, 2009
using System;
using System.Collections.Generic;
public class CommaQuibbling
{
static string Extend(string concat, string separator, string value)
{
if ( value == null)
return concat;
if ( concat.Length > 1)
return concat + separator + value;
return concat + value;
}
static string Concat(IEnumerable<string> strings)
{
var concat = "{";
var iter = strings.GetEnumerator();
string lastItem = null;
while (iter.MoveNext())
{
concat = Extend(concat, ", ", lastItem);
lastItem = iter.Current;
}
concat = Extend(concat," and ", lastItem);
return concat + "}";
}
static void TestConcat(IEnumerable<string> strings, string expected)
{
var value = Concat(strings);
WL("{0} == {1} => {2}", expected, value, expected == value);
}
public static void Main()
{
TestConcat(new string[]{}, "{}");
TestConcat(new []{"ABC"}, "{ABC}");
TestConcat(new []{"ABC", "DEF"}, "{ABC and DEF}");
TestConcat(new []{"ABC", "DEF", "G", "H"}, "{ABC, DEF, G and H}");
Console.ReadLine();
}
static void WL(object text, params object[] args)
{
Console.WriteLine(text.ToString(), args);
}
}Anonymous
April 15, 2009
Here is my single pass solution public string MakeNonOxfordList(IEnumerable<string> values) { IEnumerator<string> enumerator = values.GetEnumerator(); string current = ""; StringBuilder output = new StringBuilder(); while(enumerator.MoveNext()) { if(output.Length>0) { output.Append(", "); } output.Append(current); current = enumerator.Current; } if(output.Length==0) { output.Append(current); } else { output.Append(" AND " + current); } return "{" + output + "}"; }Anonymous
April 15, 2009
I went after two different options - one for utmost clarity and one for performance. I'd use the performant one if I was exposing this method as a public because I have no idea of what's being enumerated. However, I suspect that in some cases the internal unsafe implementation of string.Join will beat the StringBuilder, making the "clarity" approach faster than the "performant" approach. If I read the source right for string.Join, it will only ever do a single allocation, while StringBuilder will do a normal growth behavior as you add to it. using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
namespace StringGame
{
class Program
{
private static string GetPrettyJoinClear(IEnumerable<string> theStrings)
{
List<string> list = theStrings.ToList();
switch (list.Count())
{
case 0:
return "{}";
case 1:
return "{" + list[0] + "}";
case 2:
return "{" + list[0] + " and " + list[1] + "}";
default:
// Faster for some cases
list[0] = "{" + list[0];
list[list.Count - 2] = string.Join(" and ", new string[] { list[list.Count - 2], list[list.Count - 1] + "}" });
return string.Join(", ", list.Take(list.Count - 1).ToArray());
// Clearer for all cases
//return
// "{" +
// string.Join(", ", list.Take(list.Count - 1).ToArray()) +
// " and " +
// list[list.Count - 1] +
// "}";
}
} private static string GetPrettyJoinOnePass(IEnumerable<string> theStrings)
{
StringBuilder sb = new StringBuilder();
sb.Append("{");
IEnumerator<string> ie = theStrings.GetEnumerator();
bool isFirst = true;
bool isSecond = true;
string oneAgo = string.Empty;
string twoAgo = string.Empty;
foreach (string current in theStrings)
{
if (isFirst)
{
oneAgo = current;
isFirst = false;
continue;
}
if (isSecond)
{
twoAgo = oneAgo;
oneAgo = current;
isSecond = false;
continue;
}
sb.Append(twoAgo);
sb.Append(", ");
twoAgo = oneAgo;
oneAgo = current;
}
if (!isFirst)
{
if (!isSecond)
{
sb.Append(twoAgo);
sb.Append(" and ");
}
sb.Append(oneAgo);
}
sb.Append("}");
return sb.ToString();
} static void Main(string[] args)
{
string[] testArray26 = new string[] {
"a", "b", "c", "d", "e",
"f", "g", "h", "i", "j",
"k", "l", "m", "n", "o",
"p", "q", "r", "s", "t",
"u", "v", "w", "x", "y", "z" };
string[] testArray3 = new string[] { "A", "B", "C" };
string [] testArray2 = new string[] { "A", "B"};
string [] testArray1 = new string[] { "A" };
string[] testArray0 = new string[] { };
RunTest(testArray0);
RunTest(testArray1);
RunTest(testArray2);
RunTest(testArray3);
RunTest(testArray26);
Console.ReadLine();
} private static void RunTest(string[] activeArray)
{
string output = GetPrettyJoinOnePass(activeArray.AsEnumerable());
string output2 = GetPrettyJoinClear(activeArray.AsEnumerable());
Console.WriteLine("Testing with string: " + string.Join("", activeArray));
Console.WriteLine("One Pass Result : " + output);
Console.WriteLine("Clear Result : " + output2 );
Console.WriteLine();
}
}
}Anonymous
April 15, 2009
Whoops, I prefer tabs over spaces ;) I also left out StringBuilder and went for string concat for simplicity's sake. // RyanAnonymous
April 15, 2009
Here's my take in F#. Using pattern matching, the code reads just like the problem specification: #light let format (words:list<string>) = let rec makeList (words: list<string>) = match words with | [] -> "" | first :: [] -> first | first :: second :: [] -> first + " and " + second | first :: second :: rest -> first + ", " + second + ", " + (makeList rest) "{" + (makeList words) + "}" and the test case: printfn "%s" (format []) printfn "%s" (format ["ABC"]) printfn "%s" (format ["ABC"; "DEF"]) printfn "%s" (format ["ABC"; "DEF"; "G"; "H"]) yields: {} {ABC} {ABC and DEF} {ABC, DEF, G and H}Anonymous
April 15, 2009
Here's my LINQ solution: public static string CommaQuibbling(IEnumerable<string> items) { Func<int, string> getSeparator = (i) => i == 0 ? string.Empty : (i == 1 ? " and " : ", "); string answer = string.Empty; return "{" + items .Reverse() .Select((s, i) => new { Index = i, Value = s }) .Aggregate(answer, (s, a) => a.Value + getSeparator(a.Index) + s) + "}"; }Anonymous
April 15, 2009
I think most solutions posted here are convoluted. I also don't like building up strings and then later editing them to conform to the rules. Some of the posted solutions are very nice, though! I thought it'd be fun to build a short but sweet LINQed solution. Assume input is in 'IEnumerable<string> strings'. int last = strings.Count() - 1; Func<string, int, string> prefixer = delegate(string s, int index) { if (index == 0) return s; if (index == last) return " and " + s; return ", " + s; }; return "{" + string.Concat(strings.Select(prefixer).ToArray()) + "}";Anonymous
April 15, 2009
using System; using System.Collections.Generic; using System.Text; using MbUnit.Framework; namespace TestApp.Fun { /// <summary> /// Solution to problem at /// http://blogs.msdn.com/ericlippert/archive/2009/04/15/comma-quibbling.aspx /// </summary> public class Comma_Quibbling { public static string Concatenate(IEnumerable<string> sequence) { string remainderFormat = "{0}, "; string secondToLastFormat = "{0} and "; string lastFormat = "{0}"; Queue<string> queue = new Queue<string>(3); StringBuilder sb = new StringBuilder("{"); foreach (string item in sequence) { queue.Enqueue(item); if (queue.Count > 2) { sb.AppendFormat(remainderFormat, queue.Dequeue()); } } if (queue.Count == 2) { sb.AppendFormat(secondToLastFormat, queue.Dequeue()); } if (queue.Count == 1) { sb.AppendFormat(lastFormat, queue.Dequeue()); } sb.Append("}"); return sb.ToString(); } [TestFixture] public class UnitTests { [Test] [Row(new string[] { }, "{}")] [Row(new string[] { "ABC" }, "{ABC}")] [Row(new string[] { "ABC", "DEF" }, "{ABC and DEF}")] [Row(new string[] { "ABC", "DEF", "G", "H" }, "{ABC, DEF, G and H}")] public void TestConcat(IEnumerable<string> sequence, string expectedResult) { string result = Concatenate(sequence); Assert.AreEqual<string>(expectedResult, result); } } } }Anonymous
April 15, 2009
This looked like a fun problem, I'm a python guy though, so tried to solve it in python. In python I would convert the input to a list too. I don't think there is any performance benefit in looping over the input data directly, as concatenating to an existing string probably causes it to be re-allocated and copied, which I think negates any benefit gained from not doing the conversion to list up-front. def comma (data): seq = list(data) end = "" if len(seq) > 1: end = " and " + seq.pop () return "{%s%s}" % (", ".join (seq), end)Anonymous
April 15, 2009
Here's mine in python, along with the matching problem statement. to iterate with oxford commas, we follow these rules; A) Every item except the last two is followed by a comma and space
B) The penultimate item is spearated by ' and '
C) The last item stands alone. Here's the python; def noOxfordComma(sequence):
queue = []
result = ""
for item in sequence:
queue.append(item)
if len(queue) > 2: result = result + queue.pop() + ", "
if len(queue) == 2: result = result + queue.pop() + " and "
if len(queue) == 1: result = result + queue.pop()
return "{" + result + "}"Anonymous
April 15, 2009
Not as elegant as some of the other solutions but I will still post it. //We need to create comma separated list but with a twist. //The last word will have AND in front of it instead of a comma. //To achieve this, we will create a new list from the original list //while putting , in front of each word, except for the first word. //For the last word, we will replace comma with AND. private static string JoinWords2(IEnumerable<string> words) { List<string> output = new List<string>(); foreach (string word in words) { string separator = output.Count == 0 ? string.Empty : ", "; output.Add(separator + word); } if(output.Count > 1) { string lastWord = output[output.Count - 1].Substring(2); //SUBSTRING will get rid of the ", " in front of the actual word. output[output.Count - 1] = " AND " + lastWord; //And we will prepend the word with AND. //We are not doing search and replace on "," because the last word could very well be "," and //search replace will break it. } return string.Format("{{{0}}}", string.Join(string.Empty, output.ToArray())); }Anonymous
April 15, 2009
My earlier solution didnt handle all the scenarios. public static void Format(string pSomeText, IEnumerable<string> pStrings) { var x = pStrings .Reverse() .Skip(1) .Reverse() .DefaultIfEmpty() .Aggregate((a, b) => a += ", " + b); var y = String.Concat(x, (pStrings.Count() > 1 ? " And " : ""), pStrings.DefaultIfEmpty().Last()); Console.WriteLine(String.Format("{0} -> {{ {1} }}", pSomeText, y)); } Format("Empty Sequence", new List<string> { }); Format("Single Item", new List<string> { "ABC" }); Format("Two Items", new List<string> { "ABC", "DEF" }); Format("> 2 Items", new List<string> { "ABC", "DEF", "G", "H" });Anonymous
April 15, 2009
// please read carefully very important comment in the next line // :) using System; using System.Text; using System.IO; using System.Collections.Generic; using System.Linq; namespace xTry { class MainClass { private static string delimiter; public static void Main() { TestConcat(new string[]{}, "{}"); TestConcat(new []{"ABC"}, "{ABC}"); TestConcat(new []{"ABC", "DEF"}, "{ABC and DEF}"); TestConcat(new []{"ABC", "DEF", "G", "H"}, "{ABC, DEF, G and H}"); } static void TestConcat(IEnumerable<string> strings, string expected) { var value = Concat(strings); Console.WriteLine("{0} == {1} => {2}", expected, value, expected == value); } static string Concat(IEnumerable<string> strings) { string res = ""; delimiter=" and "; if(strings.Count()>0) { res=strings.Reverse().Aggregate((workingSentence, next) => next + GetDelimiter() + workingSentence); } return "{"+res+"}"; } private static string GetDelimiter() { string tmp=delimiter; delimiter=", "; return tmp; } } }Anonymous
April 15, 2009
I don't see any Ruby here. This is unacceptable. And where are everyone's tests? module Enumerable def bracketed_english_join out = inject([]) { |array, item| array + [item, ', '] } '{' + case out.length when 0 then '' when 2 then out[0] else ( out[out.length - 3] = ' and '; out[0, out.length - 1].join ) end + '}' end end if FILE == $0 require 'test/unit' class BracketedEnglishJoinTestCase < Test::Unit::TestCase def test_empty_returns_empty_string assert_equal('{}', [].bracketed_english_join) end def test_single_returns_item_only assert_equal( '{ABC}', ['ABC'].bracketed_english_join ) end def test_dual_returns_and_separated assert_equal( '{ABC and DEF}', ['ABC', 'DEF'].bracketed_english_join ) end def test_many_returns_comma_then_and_separated assert_equal( '{ABC, DEF, G and H}', ['ABC', 'DEF', 'G', 'H'].bracketed_english_join ) end end endAnonymous
April 15, 2009
Oops, Fernando Nicolet already put together something very similar :(Anonymous
April 15, 2009
Great solutions. But If I have to pick one for readability alone (ignoring efficiency), I will pick Oliver's solution. His code is shortest and crystal clear in what it does.Anonymous
April 15, 2009
The comment has been removedAnonymous
April 15, 2009
The comment has been removedAnonymous
April 15, 2009
The comment has been removedAnonymous
April 15, 2009
A Javascript solution (assuming an Array input) so Eric won't forget his roots: function makeList(array) { var a = array.concat(), last = a.pop() || ""; return "".concat( "{", a.length? a.join(", ")+" and " : "", last, "}" ); } Sure it could be made longer and clearer, but where's the fun in that?Anonymous
April 15, 2009
Actually, the last two code lines should have been: quibHelper (firstWord:words) True = firstWord ++ (quibHelper words False) quibHelper (middleWord:words) False = ", " ++ middleWord ++ (quibHelper words False) (indeed, better named variables convey intention better)Anonymous
April 15, 2009
I have two. Once is pretty readable, but does not scale to huge lists. The other uses straight enumeration and doesn't take a local copy. [Test] public void TestConvertToList() { var input1 = new string[] { }; var input2 = new string[] { "ABC" }; var input3 = new string[] { "ABC", "DEF" }; var input4 = new string[] { "ABC", "DEF", "G", "H" }; var expectedOutput1 = "{}"; var expectedOutput2 = "{ABC}"; var expectedOutput3 = "{ABC and DEF}"; var expectedOutput4 = "{ABC, DEF, G and H}"; Assert.AreEqual(expectedOutput1, ConvertToBracedEnglishSentence_Readable(input1)); Assert.AreEqual(expectedOutput2, ConvertToBracedEnglishSentence_Readable(input2)); Assert.AreEqual(expectedOutput3, ConvertToBracedEnglishSentence_Readable(input3)); Assert.AreEqual(expectedOutput4, ConvertToBracedEnglishSentence_Readable(input4)); Assert.AreEqual(expectedOutput1, ConvertToBracedEnglishSentence_EfficientForLargeLists(input1)); Assert.AreEqual(expectedOutput2, ConvertToBracedEnglishSentence_EfficientForLargeLists(input2)); Assert.AreEqual(expectedOutput3, ConvertToBracedEnglishSentence_EfficientForLargeLists(input3)); Assert.AreEqual(expectedOutput4, ConvertToBracedEnglishSentence_EfficientForLargeLists(input4)); } /// <summary> /// This method assumes that the input collection is not huge. /// If it is huge, creating a local copy of the collection /// will cost time and memory. /// This method is very readable. /// </summary> /// <param name="inputCollection"></param> /// <returns></returns> private static string ConvertToBracedEnglishSentence_Readable(IEnumerable<string> inputCollection) { // Convert to fixed list so that we know the count List<string> items = new List<string>(inputCollection); StringBuilder sentence = new StringBuilder(); for (int i = 0; i < items.Count; i++) { // All items except the last two if (i < items.Count - 2) { sentence.AppendFormat("{0}, ", items[i]); } // The second to last item else if (i == items.Count - 2) { sentence.AppendFormat("{0} and ", items[i]); } // The last item else { sentence.Append(items[i]); } } // Add the braces around the result return string.Format("{{{0}}}", sentence.ToString()); } /// <summary> /// This method does not require a local copy of the input /// collection. That should make it faster and less /// memory hungry for large input lists. /// It is a but less readable, but still fairly clear: /// Start with the open brace "{" then /// For each item in the inputCollection, /// if it's not the first item, append ", " (and remember where we put it), then /// append the item to the sentence. /// After all items have been added, add the closing brace "}" /// If we inserted a comma, replace the last one with " and ". /// Then return the sentence surrounded by braces. /// </summary> /// <param name="inputCollection"></param> /// <returns></returns> private static string ConvertToBracedEnglishSentence_EfficientForLargeLists(IEnumerable<string> inputCollection) { int indexOfLastCommaInsert = -1; bool firstItem = true; StringBuilder sentence = new StringBuilder(); sentence.Append("{"); foreach (string item in inputCollection) { if (!firstItem) { indexOfLastCommaInsert = sentence.Length; sentence.Append(", "); } firstItem = false; sentence.Append(item); } sentence.Append("}"); if (indexOfLastCommaInsert >= 0) { sentence.Replace(", ", " and ", indexOfLastCommaInsert, 2); } return sentence.ToString(); }Anonymous
April 15, 2009
Thanks for great posts, Eric! As I saw this question, I came up with all kinds of efficient solutions, but variants of them have already been posted here. So I tried to think of the most unusual solution in C# that no one else would think of. My solution is inefficient and cryptic, it's presented only as a mind game, and of course I would never write this kind of code in my projects. Nevertheless - it is very cool (it's recursive!). See if you can understand why and how it works: private static string JoinWords(IEnumerable<string> e) { IEnumerator<string> en = e.GetEnumerator(); en.Reset(); string dummy; return "{" + (en.MoveNext() ? JoinRecursive(en, out dummy) : string.Empty) + "}"; } private static string JoinRecursive(IEnumerator<string> en, out string sep) { string result = en.Current; if (en.MoveNext()) { string rest = JoinRecursive(en, out sep); result += sep + rest; sep = ", "; } else { sep = " and "; } return result; }Anonymous
April 15, 2009
My serious attempt looked much like Olivier's, except with a "l.Take(l.Count() - 1)" in place of GetRange( ), which is probably inferior. Then I started looking for the tersest possible solution. I haven't been able to top this: public string NOxfordComma_Silly(IEnumerable<string> l) { var i = 0; return "{" + l.Reverse().Aggregate("", (a, b) => b + new[] {"", " and ", ", "}[Math.Min(i++, 2)] + a) + "}"; } Obviously pretty crummy from an efficiency and readability standpoint, but it's always fun to find perverse misuses for the LINQ extension methods. :)Anonymous
April 15, 2009
Haskell: format :: [String] -> String format l = "{"++(sep l)++"}" where sep [] = "" sep [a] = a sep [a,b] = a++" and "++b sep (a:l) = a++", "++(sep l)Anonymous
April 15, 2009
Some interesting solutions here, but nothing worthy of enterprise production code. What's the deal... I don't even see any factories or XML! Admittedly I don't have a lot of time, but here's my first pass at a real enterprise solution. I'll add an XML-serving web service later. Should work with VS2008/.NET 3.5. using System; using System.Collections.Generic; using System.Linq; using System.Text; namespace Scratch { class Program { static void Main(string[] args) { string[] input = { "ABC", "DEF", "G", "H" }; Console.WriteLine(EnumerableStringFormatter.FormatInput(input)); } } class EnumerableStringFormatter { public static string FormatInput(IEnumerable<string> input) { var words = new List<string>(); var separators = new List<string>(); var pairs = new List<WordFactory.WordSeparatorPair>(); // get words foreach (var word in WordFactory.GetWord(input)) words.Add(word); // get separators foreach (var separator in WordFactory.GetSeparator(input)) separators.Add(separator); // combine them for (int i = 0; i < words.Count; i++) pairs.Add(new WordFactory.WordSeparatorPair(words[i], separators[i])); // convert to a string StringBuilder sb = new StringBuilder("{"); foreach (var pair in pairs) sb.Append(String.Format("{0}{1}", pair.Separator, pair.Word)); return sb.Append("}").ToString(); } class WordFactory { public static IEnumerable<string> GetWord(IEnumerable<string> input) { foreach (var s in input) yield return s; } public static IEnumerable<string> GetSeparator(IEnumerable<string> input) { yield return ""; var a = input.Skip(1).TakeWhile(s => s != input.Last()); foreach (var s in a) yield return ", "; yield return " and "; } public class WordSeparatorPair { private string _word; private string _separator; public string Word { get { return _word; } } public string Separator { get { return _separator; } } public WordSeparatorPair(string word, string separator) { _word = word; _separator = separator; } } } } } PS: I'm really not as familiar with LINQ as I'd like to be, so there are probably better^Wworse ways to do this.Anonymous
April 15, 2009
PPS: For what it's worth, here was my first blind attempt at solving the problem. As with others, I usually prefer a simple and straightforward solution compared to a clever shorter one. (However, I do enjoy reading sneaky clever code -- just not at work). static string FormatEnumerableStrings(IEnumerable<string> input) { var sb = new StringBuilder("{"); var strings = input.ToList(); int count = strings.Count; if (count > 0) { sb.Append(strings[0]); for (int i = 1; i < count - 1; i++) { sb.Append(", "); sb.Append(strings[i]); } if (count > 1) { sb.Append(" and "); sb.Append(strings[count - 1]); } } return sb.Append("}").ToString(); }Anonymous
April 15, 2009
I went for as terse as possible with Linq. I don't preserve the order, but I don't think that was a requirement. It is also fairly effecient as it only revisits the first couple items. static string FormatStrings(IEnumerable<string> strings) { return string.Format("{{{0}}}", string.Concat( strings.Skip(2).Select((x) => x + ", ").Concat( strings.Skip(1).Take(1).Select((y) => y + " and ")).Concat( strings.Take(1).Select((z) => z)).ToArray())); }//methodAnonymous
April 15, 2009
I realized that given the rules the order probably does matter, in which case this version, which is much less efficient but almost equally terse, works. static string FormatStrings(IEnumerable<string> input) { var reordered = input.Reverse().Take(2).Concat(input.Reverse().Skip(2).Reverse()); return string.Format("{{{0}}}", string.Concat( reordered.Skip(2).Select((x) => x + ", ").Concat( reordered.Skip(1).Take(1).Select((y) => y + " and ")).Concat( reordered.Take(1).Select((z) => z)).ToArray())); }//methodAnonymous
April 15, 2009
Minor correction for Hristo's F# pattern matching solution, which breaks for lists of odd length > 1: let format (words : list<string>) = let rec makeList (words : list<string>) = match words with | [] -> "" | first :: [] -> first | first :: second :: [] -> first + " and " + second | first :: rest -> first + ", " + (makeList rest) "{" + (makeList words) + "}"Anonymous
April 15, 2009
Haskell. It practically reads like the problem statement! I guess it does assume you are using arrays. I don't know how to do enums yet, I'll try later maybe. inner :: [String] -> String inner [] = "" inner [a] = a inner [a,b] = a ++ " and " ++ b inner (a:rest) = a ++ ", " ++ inner rest formatString :: [String] -> String formatString a = "{" ++ inner a ++ "}"Anonymous
April 15, 2009
My solution: static string Join(IEnumerable<string> words) { StringBuilder buffer = new StringBuilder(); buffer.Append("{"); bool isFirst = true; int counter = 0; int count = words.Count(x => true); foreach (string word in words) { if (isFirst) buffer.Append(" " + word); else if (counter == count - 1) buffer.Append(" and " + word); else buffer.Append(", " + word); isFirst = false; counter++; } buffer.Append(" }"); return buffer.ToString(); } static void Main(string[] args) { Console.WriteLine(Join(new string[] { })); Console.WriteLine(Join(new string[] { "ds" })); Console.WriteLine(Join(new string[] { "ds", "sdf" })); Console.WriteLine(Join(new string[] { "ds", "sdf", "sdfs" })); Console.WriteLine(Join(new string[] { "ds", "sdf", "sdfs", "rty" })); }Anonymous
April 15, 2009
My solution in Java: public static void main(String[] args) { ArrayList<String> list = new ArrayList<String>(); list.add("ABC"); list.add("DEF"); list.add("GHI"); list.add("JKL"); Iterator<String> it = list.iterator(); System.out.println("{" + print(it,false).replace(", and", " and") + "}"); } private static String print(Iterator<String> iter, boolean nonFirst) { if(iter.hasNext()){ String curr1 = iter.next(); if(!iter.hasNext()) return (nonFirst?" and ":"") + curr1; else return curr1 + "," + print(iter, true); } return ""; }Anonymous
April 15, 2009
Here is a perl solution (with tests): #!/net/bin/perl use strict; use warnings; use Test::More 'tests' => 4; is( concat(), '{}', ); is( concat('ABC'), '{ABC}', ); is( concat('ABC', 'DEF'), '{ABC and DEF}', ); is( concat('ABC', 'DEF', 'G', 'H'), '{ABC, DEF, G and H}', ); exit; ################# sub concat { my @parts = @; if ( not @parts ) { return '{}'; } if ( scalar @parts < 2 ) { return '{' . $parts[0] . '}'; } my $last = pop @parts; return '{' . join( ', ', @parts ) . ' and ' . $last .'}'; } I wrote this before reading the comments and was pleasantly surprised to see that Olivier Leclant had used the same technique. A different version that simply joins with comma and then substitutes the last comma: sub concat { my $string = join ', ', @; # assuming that parts are just capital ascii letters $string =~ s{ , s ([A-Z]+) z }{ and $1}smx; return '{' . $string . '}'; } But note the massive assumption of what the data is.Anonymous
April 15, 2009
The comment has been removedAnonymous
April 15, 2009
The comment has been removedAnonymous
April 15, 2009
The comment has been removedAnonymous
April 15, 2009
private string StringAte(IEnumerable<string> strings) { StringBuilder list = new StringBuilder("{"); int lastComma = -1; string comma = string.Empty; foreach (string s in strings) { list.AppendFormat("{0}{1}", comma, s); if (comma.Length < 1) { comma = ", "; } else { lastComma = list.Length - s.Length - comma.Length; } } if (lastComma > 0) { list.Replace(comma, " AND ", lastComma, comma.Length); } list.Append("}"); return list.ToString(); }Anonymous
April 15, 2009
class Program { static void Main(string[] args) { Console.WriteLine(AppendWords(null)); Console.WriteLine(AppendWords(string.Empty)); Console.WriteLine(AppendWords("ABC")); Console.WriteLine(AppendWords("ABC", "DEF")); Console.WriteLine(AppendWords("ABC", "DEF", "GHI")); Console.ReadKey(); } static string AppendWords(params string[] words) { if (words == null || words.Length == 0) { return "{}"; } StringBuilder builder = new StringBuilder(); int counter = 0; int wordCountToAppend = 0; builder.Append("{"); do { builder.Append(words[counter]); counter++; wordCountToAppend = words.Length - counter; if (counter >= 1 && wordCountToAppend >= 2) { builder.Append(", "); } else if (wordCountToAppend == 1) { builder.Append(" and "); } } while (counter < words.Length); builder.Append("}"); return builder.ToString(); } }Anonymous
April 15, 2009
The comment has been removedAnonymous
April 15, 2009
private string BuildString(IEnumerable<string> list) { StringBuilder sb = new StringBuilder("{"); string values = string.Join(",", list.ToArray()); int pos = values.LastIndexOf(','); if(pos > -1) { values = values.Remove(pos, 1); values = values.Insert(pos, " and "); } sb.Append(values); sb.Append("}"); return sb.ToString(); }Anonymous
April 15, 2009
So my solution was to just join the strings with a comma, and then find the last comma and replace it with an " and ". This satisfies the requirements as stated in the original question.Anonymous
April 15, 2009
using System.Collections.Generic; using System.Linq; namespace CommaQuibbling { internal class Translator { public string Translate(IEnumerable<string> items) { return "{" + Join(items) + "}"; } private static string Join(IEnumerable<string> items) { var leadingItems = LeadingItemsFrom(items); var lastItem = LastItemFrom(items); return JoinLeading(leadingItems) + lastItem; } private static IEnumerable<string> LeadingItemsFrom(IEnumerable<string> items) { return items.Reverse().Skip(1).Reverse(); } private static string LastItemFrom(IEnumerable<string> items) { return items.Reverse().FirstOrDefault(); } private static string JoinLeading(IEnumerable<string> items) { if (items.Any() == false) return ""; return string.Join(", ", items.ToArray()) + " and "; } } }Anonymous
April 15, 2009
OK, writing from Australia, which is why I'm so behind everyone else. I haven't looked at other people's solution, and I'm sure I'm just replicating everyone else's code, but here goes: /// <summary> /// Formats an enumeration of strings using commas as /// dictated by English grammar rules. /// </summary> /// <example> /// {} -> "{}" /// {"ABC"} -> "{ABC}" /// {"ABC", "DEF"} -> "{ABC and DEF}" /// {"ABC", "DEF", "G", "H"} -> "{ABC, DEF, G and H}" /// </example> /// <param name="strings">Enumeration of string to join</param> /// <returns>A comma seperated list of the strings with the last /// two elements seperated by 'and'</returns> public string EnglishJoin(IEnumerable<string> strings) { Queue<string> stringQueue = new Queue<string>(); StringBuilder sb = new StringBuilder(); sb.Append("{"); foreach (string s in strings) { stringQueue.Enqueue(s); if (stringQueue.Count > 2) { sb.Append(stringQueue.Dequeue()); sb.Append(", "); } } // Last two string seperated by ' and ' if (stringQueue.Count > 0) { sb.Append(stringQueue.Dequeue()); if (stringQueue.Count > 0) { sb.Append(" and "); sb.Append(stringQueue.Dequeue()); } } sb.Append("}"); return sb.ToString(); }Anonymous
April 15, 2009
Why not do it backward? Read the enumerable into a stack and build the output keeping an index: index == 0 : result = item index == 1 : result = item & " and " & result index > 1 : result = item & ", " & result Actually there is no point in lazy enumeration, as a complete result requires the whole range to be enumerated and a partial result doesn't seem to be worth much. However, the stack in this solution requires memory allocation of (size of the enumerable * size of an object reference). To avoid this one can apply a peeking mechanism to find out when one is at index last - 2. something like result.Append("{"); if (e.MoveNext()) { result.Append(e.Current); if (e.MovNext()) { last = e.Current; while (e.MoveNext()) { result.Append(", "); result.Append(last); last = e.Current; } result.Append(" and "); result.Append(last); } } result.Append("}");Anonymous
April 15, 2009
public static string WeirdString(IEnumerable<string> original) { // I found out that the StringBuilder is WAY faster than just "+" StringBuilder sb = new StringBuilder("{}"); IEnumerable<string> reversed = original.Reverse(); // Now the last element is the first bool andRatherThanComma = true; bool moreThanOne = false; foreach (string s in reversed) { if (moreThanOne) { // If this is NOT the last string in the original list and the first one in the reversed list... sb.Insert(andRatherThanComma ? " and " : ", ", 1); andRatherThanComma = false; } else moreThanOne = true; sb.Insert(s, 1); // insert the string after the first "{" : if it's the fisrt and the last one, we'll get {string} } return sb.ToString(); }Anonymous
April 15, 2009
Hey Eric, why don't you post this on stackoverflow and let the community bubble up the best answers and then you can dissect the best ones on your blog?Anonymous
April 15, 2009
/// <param name="delimiter">The delimeter placed before all elements except the first and last.</param> /// <param name="lastDelimiter">The delimiter placed before the last element.</param> /// <returns>an enumeration with the original elements interleaved with delimiters</returns> public static IEnumerable<T> Interleave<T>(this IEnumerable<T> baseEnumeration, T delimiter, T lastDelimiter) { using (var iter = baseEnumeration.GetEnumerator()) { // guard clause for empty source if (!iter.MoveNext()) yield break; //first iteration of loop is unrolled because the first item does not have a delimiter yield return iter.Current; if (!iter.MoveNext()) yield break; //This is a while loop with a break in the middle. I reuse the loop termination test to decide which delimiter to // use, thus the body of the loop gets repeated. while (true) { T item = iter.Current; if (iter.MoveNext()) { yield return delimiter; yield return item; } else { yield return lastDelimiter; yield return item; yield break; } } } } I already had a concatenate a list to a string method. /// <summary> /// Interleaves the specified enumerable with the given delimiter /// </summary> /// <typeparam name="T">Basis type of the enumeration</typeparam> /// <param name="enumerableToInterleave">The enumerable to interleave.</param> /// <param name="delimiter">The delimiter.</param> /// <returns>Original enumeration inteleaved with the given delimiter</returns> public static IEnumerable<T> Interleave<T>(this IEnumerable<T> enumerableToInterleave, T delimiter) { return Interleave<T>(enumerableToInterleave, delimiter, delimiter); } and then the answer is trivial. public static string EnglishList(this IEnumerable<String> input) { return "{" } input.Interleave(", ", " and ").ConcatenateStrings() + "}"; } This algorithm only iterates the enumerator once, and requires a single element buffer (the item variable) to detect the last element. I think separating the inteleave problem from the concatenation problem makes the code read very clear in spite of the somewhat "clever" loop with an exit in the middle which is used in the interleave method.Anonymous
April 15, 2009
So I should have read the specs a little further and realized that I needed to use the IEnumerable<string> methods in order to implement this algorightm. Just to make sure my solution is still considered I made some adjustments. Please see my revised version below.
class Program { static void Main(string[] args) { Console.WriteLine(AppendWords(null)); Console.WriteLine(AppendWords(string.Empty)); Console.WriteLine(AppendWords("ABC")); Console.WriteLine(AppendWords("ABC", "DEF")); Console.WriteLine(AppendWords("ABC", "DEF", "GHI")); Console.WriteLine(AppendWords("ABC", "DEF", "GHI", "JKL")); Console.ReadKey(); } static string AppendWords(params string[] words) { return AppendWordsInternal(words); } static string AppendWordsInternal(IEnumerable<string> words) { if (words == null) { return "{}"; } StringBuilder builder = new StringBuilder(); int appendedWords = 0; int totalWords = words.Count(); int wordsRemaining = 0; builder.Append("{"); IEnumerator<string> enumerator = words.GetEnumerator(); while (enumerator.MoveNext()) { builder.Append(enumerator.Current); appendedWords++; wordsRemaining = totalWords - appendedWords; if (appendedWords >= 1 && wordsRemaining >= 2) { builder.Append(", "); } else if (wordsRemaining == 1) { builder.Append(" and "); } } builder.Append("}"); return builder.ToString(); } }
Anonymous
April 15, 2009
public static string Join(IEnumerable<string> strings) { return JoinHelper(strings).Aggregate(new StringBuilder(), (sb, s) => sb.Append(s), sb => sb.ToString()); } private static IEnumerable<string> JoinHelper(IEnumerable<string> strings) { yield return "{"; string current = null; bool first = true; foreach (string item in strings) { if (current != null) { if (!first) { yield return ", "; } first = false; yield return current; } current = item; } if(current != null) { if (!first) { yield return " and "; } yield return current; } yield return "}"; }Anonymous
April 15, 2009
public static void Main() { PrintFriendlyArray(new string[]{"ABC"}); PrintFriendlyArray(new string[] {"ABC", "DEF"}); PrintFriendlyArray(new string[] {"ABC", "DEF","G","H"}); PrintFriendlyArray(new string[]{"", ",","}"}); } public static void PrintFriendlyArray(IEnumerable<string> strings) { StringBuilder friendlyString = new StringBuilder(); friendlyString.Append("{"); friendlyString.Append(strings.Aggregate((current, next) => (strings.LastOrDefault().Equals(next)?current + " and " + next: current + "," + next))); friendlyString.Append("}"); Console.WriteLine(friendlyString); }Anonymous
April 15, 2009
This is my first post ever, Eric. Apart from handling null, wouldn't this program work in all scenarios? I also see that my naming is not consistent. The method name should have been PrintFriendlyString instead of PrintFriendlyArray, right? This is the most straightforward and semantically nearest program I could think of. Reply if you find time.Anonymous
April 15, 2009
class Program { static void Main(string[] args) { Console.WriteLine(Lippertize(new string[0])); Console.WriteLine(Lippertize(new string[]{"ABC"})); Console.WriteLine(Lippertize(new string[]{"ABC", "DEF"})); Console.WriteLine(Lippertize(new string[]{"ABC", "DE, F", "G", "H"})); Console.ReadKey(); } static string Lippertize(IEnumerable<string> source) { return "{" + Concat(source, ", ", " and ") + "}"; } static string Concat(IEnumerable<string> source, string separator, string lastSeparator) { var firstItem = true; var gotTwo = false; var lastSeparatorPos = 0; var sb = new StringBuilder(); var quoted = from s in source where !string.IsNullOrEmpty(s) select s; foreach (var item in quoted) { if (!firstItem) { gotTwo = true; lastSeparatorPos = sb.Length; sb.Append(separator); } else firstItem = false; sb.Append(item); } if (gotTwo) // step back and replace the last separator with the correct one: { sb.Remove(lastSeparatorPos, separator.Length); sb.Insert(lastSeparatorPos, lastSeparator); } return sb.ToString(); } }Anonymous
April 15, 2009
Quite verbose (even without the unit tests). But works. using System; using System.Collections.Generic; using System.Linq; using System.Text; using NUnit.Framework; namespace CommaQuibble { [TestFixture] public class QuibblerFixture { [Test] public void EmptyList() { IEnumerable<string> list = new[] {""}; string quibbled = GetQuibbled(list); Assert.AreEqual("{}", quibbled); } private static string GetQuibbled(IEnumerable<string> list) { Quibbler quibbler = new Quibbler(); return quibbler.Quibble(list); } [Test] public void OneItemInList() { IEnumerable<string> list = new[] {"ABC"}; string quibbled = GetQuibbled(list); Assert.AreEqual("{ABC}", quibbled); } [Test] public void TwoItemsInList() { string[] list = new[] { "ABC", "DE" }; string quibbled = GetQuibbled(list); Assert.AreEqual("{ABC and DE}", quibbled); } [Test] public void ThreeItemsInList() { string[] list = new[] { "ABC", "DE", "ZYXWV" }; string quibbled = GetQuibbled(list); Assert.AreEqual("{ABC, DE and ZYXWV}", quibbled); } [Test] public void ManyMany() { string[] list = new[] { "ABC", "DE", "ZYXWV", "FG", "UT", "HI", "SR", "JK", "QP", "LM", "NO" }; string quibbled = GetQuibbled(list); Assert.AreEqual("{ABC, DE, ZYXWV, FG, UT, HI, SR, JK, QP, LM and NO}", quibbled); } [Test] public void ViacheslavIvanov() { string[] list = new[] { "", ",", "}" }; string quibbled = GetQuibbled(list); Assert.AreEqual("{, , and }}", quibbled); } } public class Quibbler { public string Quibble(IEnumerable<string> enumerable) { StringBuilder builder = new StringBuilder("{"); string last = enumerable.Last(); string first = enumerable.First(); if(first == last) { builder.Append(first); } else if(first != last) { IEnumerable<string> rest = enumerable.Except(new[] { last }); string penultimate = rest.Last(); QuietStack stacked = new QuietStack(rest.Reverse()); while (stacked.Peek() != null) { string current = stacked.Pop(); builder.Append(current); if (current != penultimate) builder.Append(", "); } builder.AppendFormat(" and {0}", last); } builder.Append("}"); return builder.ToString(); } } class QuietStack { private readonly Stack<string> m_Stack; public QuietStack(IEnumerable<string> collection) { m_Stack = new Stack<string>(collection); } public string Pop() { return m_Stack.Pop(); } public string Peek() { try { return m_Stack.Peek(); } catch (InvalidOperationException) { return null; } } } }Anonymous
April 15, 2009
How to misuse LINQ: public void RunTest()
{
Console.WriteLine(new Class1().GetResult(new string[] { }));
Console.WriteLine(new Class1().GetResult(new[] { "ABC" }));
Console.WriteLine(new Class1().GetResult(new[] { "ABC", "DEF" })); Console.WriteLine(new Class1().GetResult(new[] { "ABC", "DEF", "GHI" }));
}
public string GetResult(IEnumerable<string> input)
{
var list = new List<string>(input);
if (list.Count == 0) return "{}";
if (list.Count == 1) return "{" + list[0] + "}";
return "{" + string.Join(", ", list.Take(list.Count - 1).ToArray()) + " and " + list[list.Count - 1] + "}";
}Anonymous
April 15, 2009
I would go with Olivier and mdefalco here. My solution was almost identical to Olivier's. I really felt the need to some sort of recursive String.Format method. However, one that is closer to my heart is: public string Join(IEnumerable<string> strings) { if (strings == null || !strings.Any()) return "{}"; var q = new Queue<string>(strings); var last = q.Dequeue(); if (q.Count == 0) return "{" + last + "}"; return "{" + string.Join(", ", q.ToArray()) + " and " + last + "}"; }Anonymous
April 15, 2009
Most of the F# posts use list pattern matching, conveniently ignoring that the input is a sequence, not a list. The following solution allows for pattern matching without incurring the cost of converting the sequence to a list. It is declarative, scales roughly linearly when used with Parallel Extensions, and uses a StringBuilder at the concatenation stage for maximum efficiency. open System open System.Text let concat_string (list: string seq) = let tripleWise = Seq.append list [null] |> Seq.scan (fun (_, previousPrevious, previous) current -> (previousPrevious, previous, current) ) (null, null, null) let contents = tripleWise.AsParallel() |> PSeq.map (function | _, null, _ -> String.Empty | null, curr, null -> curr | null, first, _ -> first | _, last, null -> sprintf " and %s" last | prev, curr, next -> sprintf ", %s" curr) let builder = StringBuilder() contents |> Seq.iter (fun item -> (builder.Append(item) |> ignore)) sprintf "{%s}" (builder.ToString())Anonymous
April 15, 2009
I like terse: string CommaQuibble(IEnumerable<string> words) { return string.Format("{{{0}}}", string.Join(", ", words.Take(words.Count() - 2).Concat(new[] { string.Join(" and ", words.Skip(words.Count() - 2).ToArray()) }).ToArray())); } (should check for null, though)Anonymous
April 15, 2009
// Using SmartEnumerable by John Skeet http://msmvps.com/blogs/jon_skeet/archive/2007/07/27/smart-enumerations.aspx public string Concatenate(IEnumerable<string> sequence) { SmartEnumerable<string> smartSequence = sequence.AsSmartEnumerable(); StringBuilder result = new StringBuilder(); result.Append("{"); foreach (var word in smartSequence) { if (!word.IsFirst && !word.IsLast) { result.Append(", " + word.Value); } else if (!word.IsFirst && word.IsLast ) { result.Append(" and " + word.Value); } else { result.Append(word.Value); } } result.Append("}"); return result.ToString(); }Anonymous
April 15, 2009
My quick solution as a full program. My goals were to stay with an imperative C# style, make the main function readable and to maintain the stream nature of IEnumerable (no multiple passes, no duplication via creating a list/array which makes the problem too simple). The trick here is the one-off enumerator that adds an IsFirst/IsLast. Its definitely a one-off class as written since it doesn't obey IEnumerable's specification -- though changing it to do so wouldn't be hard. But you know what they say about code that's not yet needed. using System; using System.Collections.Generic; using System.Linq; using System.Text; using System.Collections; namespace MakeFancyString { class SpecialEnumerable : IEnumerable, IEnumerator { private IEnumerator<String> source; private int itemNumber; public SpecialEnumerable(IEnumerable<String> source) { this.source = source.GetEnumerator(); Reset(); } public IEnumerator GetEnumerator() { return this; } public bool IsFirst { get { return itemNumber == 0; } } public bool IsLast { get; private set; } public object Current { get; private set; } public bool MoveNext() { if (IsLast) return false; Current = source.Current; ++itemNumber; IsLast = !source.MoveNext(); return true; } public void Reset() { source.Reset(); Current = null; itemNumber = -1; IsLast = !source.MoveNext(); } } class Program { static String MakeList(IEnumerable<String> input) { StringBuilder builder = new StringBuilder(); builder.Append("{"); var enumerable = new SpecialEnumerable(input); foreach (String item in enumerable) { if (enumerable.IsFirst) ; else if (enumerable.IsLast) builder.Append(" and "); else builder.Append(", "); builder.Append(item); } builder.Append("}"); return builder.ToString(); } static void Main(string[] args) { Console.WriteLine(MakeList(new String[] { })); // {} Console.WriteLine(MakeList(new String[] { "ABC" })); // {ABC} Console.WriteLine(MakeList(new String[] { "ABC", "DEF", })); // {ABC and DEF} Console.WriteLine(MakeList(new String[] { "ABC", "DEF", "GHI" })); // {ABC, DEF and GHI} Console.WriteLine(MakeList(new String[] { "ABC", "DEF", "GHI", "JKL" })); // {ABC, DEF, GHI and JKL} } } }Anonymous
April 15, 2009
Attempting to get this to format properly... module Enumerable def bracketed_english_join out = inject([]) { |array, item| array + [item, ', '] } '{' + case out.length when 0 then '' when 2 then out[0] else ( out[out.length - 3] = ' and '; out[0, out.length - 1].join ) end + '}' end end if FILE == $0 require 'test/unit' class BracketedEnglishJoinTestCase < Test::Unit::TestCase def test_empty_returns_empty_string assert_equal('{}', [].bracketed_english_join) end def test_single_returns_item_only assert_equal( '{ABC}', ['ABC'].bracketed_english_join ) end def test_dual_returns_and_separated assert_equal( '{ABC and DEF}', ['ABC', 'DEF'].bracketed_english_join ) end def test_many_returns_comma_then_and_separated assert_equal( '{ABC, DEF, G and H}', ['ABC', 'DEF', 'G', 'H'].bracketed_english_join ) end end endAnonymous
April 15, 2009
I'd serialize the input into xsd conforming xml and use xsl like always :). Xsd available upon request... <?xml version="1.0" encoding="utf-8"?> <xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform" xmlns:msxsl="urn:schemas-microsoft-com:xslt" exclude-result-prefixes="msxsl"> <xsl:output method="text" indent="yes"/> <xsl:template match="/"> <xsl:value-of select="'{'"/> <xsl:apply-templates select="/root/word"/> <xsl:value-of select="'}'"/> </xsl:template> <xsl:template match="word"> <xsl:choose> <xsl:when test="position() = 1"> <xsl:value-of select="."/> </xsl:when> <xsl:when test="position() = last()"> <xsl:value-of select="concat(' and ', .)"/> </xsl:when> <xsl:otherwise> <xsl:value-of select="concat(' , ', .)"/> </xsl:otherwise> </xsl:choose> </xsl:template> </xsl:stylesheet>Anonymous
April 15, 2009
I guess this solution has already been posted. .Net 2.0 Public Function Commate(ByVal Items As IEnumerable(Of String)) As String Dim ie As IEnumerator(Of String) = Items.GetEnumerator() Dim ls As New List(Of String) While ie.MoveNext() ls.Add(ie.Current) End While Dim sb As New System.Text.StringBuilder() sb.Append("{") If ls.Count > 0 Then Dim beforeAnd As String = String.Join(", ", ls.ToArray(), 0, ls.Count - 1) If ls.Count > 1 Then sb.Append(String.Join(" and ", New String() {beforeAnd, ls(ls.Count - 1)})) Else sb.Append(ls(ls.Count - 1)) End If End If sb.Append("}") Return sb.ToString() End FunctionAnonymous
April 15, 2009
The problem is really deciding while enumerating, when you are at the penultimate item in the list, presumably without counting. Here's my solution: static class Extensions { public static List<T> IntersperseToList<T>(this IEnumerable<T> values, T delimeter) { List<T> res = new List<T>(); bool first = true; foreach (var item in values) { if (!first) { res.Add(delimeter); } res.Add(item); first = false; } return res; } public static string Join(this IEnumerable<string> values) { return String.Join(String.Empty, values.ToArray()); } } static string SolveIt(IEnumerable<string> values) { List<string> res = values.IntersperseToList(", "); if (res.Count >= 2) { res[res.Count - 2] = " and "; } return "{" + res.Join() + "}"; } static void Main(string[] args) { var vals = Enumerable.Range(1, 10001).Select(i => i.ToString()); Console.WriteLine(SolveIt(vals)); Console.WriteLine(SolveIt(new string[0])); Console.WriteLine(SolveIt(new string[] { "ABC" })); Console.WriteLine(SolveIt(new string[] { "ABC", "DEF" })); Console.WriteLine(SolveIt(new string[] { "AA", "BBB", "CC", "H" })); }Anonymous
April 15, 2009
Having read through the various solutions I think Jon nailed it pretty much straight away as far as the actual implementation was concerned. In terms of clearly expressing the semantic its pretty good too however I was initially confused by the first word to be added to the output being the penultimate word. Only having analysed the code more was it clear that all words will pass through penultimate and get added to the the builder before being replaced with tne next candidate to be the penultimate. I know the comments kinda indicate this but a small tweak changing the variable name 'penultimate' to 'current' would likely have not lead to that confusion in the first place.Anonymous
April 15, 2009
class Program { static void Main() { AreEqual(Stringify(new string[] {}), "{}"); AreEqual(Stringify(new [] {"ABC" }), "{ABC}"); AreEqual(Stringify(new[] { "ABC", "DEF" }), "{ABC and DEF}"); AreEqual(Stringify(new[] { "ABC", "DEF", "G", "H" }), "{ABC, DEF, G and H}"); } static string Stringify(IEnumerable<string> sequence) { return "{" + string.Join(", ", sequence.Reverse().Skip(2).Reverse().ToArray()) + (sequence.Count()>2?", ":string.Empty) + string.Join(" and ", sequence.Reverse().Take(2).Reverse().ToArray()) + "}"; } static void AreEqual(string actual, string expected) { if (actual != expected) throw new Exception(actual + "!=" + expected); } }Anonymous
April 15, 2009
#r "FSharp.PowerPack.dll" let format (words : seq<string>) = let rec format (words : LazyList<string>) acc = match words with | LazyList.Nil -> string.Empty | LazyList.Cons(first, LazyList.Nil) -> first | LazyList.Cons(first, LazyList.Cons(second, LazyList.Nil)) -> acc + first + " and " + second | LazyList.Cons(first, rest) -> acc + first + ", " |> format rest let listOfWords = LazyList.of_seq words "{" + (format listOfWords string.Empty) + "}" ["ABC"; "DEF"; "G"; "H" ] |> format ["ABC"; "DEF" ] |> format ["ABC"] |> format [] |> formatAnonymous
April 15, 2009
Here's mine! public string Format(IList<string> items) { string separator = items.Count <= 1 ? "" : " and "; string allButLast = string.Join(", ", items.TakeWhile((t, i) => i < items.Count - 1).ToArray()); return string.Format("{{{0}{1}{2}}}", allButLast, separator, items.LastOrDefault()); } public string Format(IEnumerable<string> items) { return Format(new List<string>(items)); }Anonymous
April 15, 2009
Ο Erik Lippert στο τελευταίο του blog post , έθεσε ένα απλό προβληματάκι. Ακολουθεί η λύση που έκαναAnonymous
April 15, 2009
Looks like I'm a bit late, but here's my F# solution: let foo sequence = let rec bar ss = match ss with | [] -> "" | [a] -> a | [a;b] -> sprintf "%s and %s" a b | a::b -> sprintf "%s, %s" a (bar b) sprintf "{%s}" (sequence |> List.of_seq |> bar) And my C# solution: public string foo(IEnumerable<string> sequence) { var stack = new Stack<string>(sequence); string result = "}"; if (stack.Count > 0) result = stack.Pop() + result; if (stack.Count > 0) result = stack.Pop() + " and " + result; while (stack.Count > 0) result = stack.Pop() + ", " + result; return "{" + result; }Anonymous
April 15, 2009
I made two versions. One is recursive and very simple. The other is using extension methods a bit more, but faily simple to read aswell. Code: class StringConcatenator { public static string Concatenate(IEnumerable<string> input) { return "{" + ConcatenateRecursive(input) + "}"; } private static string ConcatenateRecursive(IEnumerable<string> input) { switch (input.Count ()) { case 0: return string.Empty; case 1: return input.First (); case 2: return input.First () + " and " + input.Last (); default: return input.First () + ", " + ConcatenateRecursive (input.Skip (1)); } } public static string Concatenate2(IEnumerable<string> input) { string result; switch (input.Count ()) { case 0: result = string.Empty; break; case 1: result = input.First(); break; default: result = input.Take (input.Count () - 1) .Aggregate ((n, t) => n + ", " + t) + " and " + input.Last(); break; } return "{" + result + "}"; } } }Anonymous
April 15, 2009
I realized my version could be slighty better. Here it is, still without use of StringBuilder ;) static string Extend(string concat, string separator, string value) { if ( value == null) return concat; if ( concat != null) return concat + separator + value; return value; } static string Concat(IEnumerable<string> strings) { string concat = null; string lastItem = null; foreach(var s in strings) { concat = Extend(concat, ", ", lastItem); lastItem = s; } concat = Extend(concat," and ", lastItem); return "{" + concat + "}"; } // RyanAnonymous
April 15, 2009
// My vote: Best is izobr version. // See: izobr (April 16, 2009 12:07 AM) // Here is little bit refactored izobr version. private static IEnumerable<string> ConcatNoOxford(IEnumerable<string> source) { yield return "{"; string prevItem = null; // Use like stack. bool hasAnyItem = false; // Target sequence has any item from source sequence. foreach (string item in source) { //TODO: Check for null/empty. if (prevItem != null) { if (hasAnyItem) { hasAnyItem = true; yield return ", "; } yield return prevItem; } prevItem = item; } if (prevItem != null) { if (hasAnyItem) { yield return " and "; } yield return prevItem; } yield return "}"; }Anonymous
April 15, 2009
private string Join( IEnumerable<string> input ) { List<string> list = new List<string>( input ); return SurroundWithBrackets( FormatElements( list ) ); } private static string FormatElements( List<string> list ) { if ( list.Count == 0 ) { return string.Empty; } if ( list.Count == 1 ) { return list[ 0 ]; } StringBuilder result = new StringBuilder(); foreach ( string item in AllExceptLastTwo( list ) ) { result.AppendFormat( "{0}, ", item ); } result.Append( FormatLastTwoItems( list ) ); return result.ToString(); } private static string SurroundWithBrackets( string input ) { return string.Format( "{{{0}}}", input ); } private static string FormatLastTwoItems( IList<string> list ) { int listCount = list.Count; return string.Format( "{0} and {1}", list[ listCount - 2 ], list[ listCount - 1 ] ); } private static IEnumerable<string> AllExceptLastTwo( List<string> list ) { //if ( list.Count < 3 ) { // return new string[0]; //} return list.GetRange( 0, list.Count - 2 ); } //Test private IEnumerable<string> input; private void AssertJoinIs( string exepcted ) { string actual = joiner.Join( input ); Console.WriteLine( "Actual: " + actual ); Assert.AreEqual( exepcted, actual ); } [Test] public void Empty() { input = new List<string>(); AssertJoinIs( "{}" ); } [Test] public void SingleElement() { input = new string[] {"ABC"}; AssertJoinIs( "{ABC}"); } [Test] public void TwoElements() { input = new string[] { "ABC", "DEF" }; AssertJoinIs( "{ABC and DEF}" ); } [Test] public void MoreElements() { input = new string[] { "ABC", "DEF", "G", "H" }; AssertJoinIs( "{ABC, DEF, G and H}" ); }Anonymous
April 15, 2009
Eric, could you please do a post about StringBuilder and how the compiler will usually introduce a call to a string.Concat() overload. Many people (even in this thread) seem to have no idea what the compiler is doing and believe they have to use StringBuilder any time they concatenate one string with another. If I had a penny for every time in a code review someone has complained that I concatenated strings using '+' instead of using StringBuilder.....Anonymous
April 15, 2009
I am a fan of the recursive solution, as it fits closely with how the problem is stated. My first attempt was very similar to Andreas Kromann's except I used ToArray to remove the need to traverse the IEnumerable each time with count and an offset to remove the need for slow array splicing. This solution uses a lot of string concatenation, which for large lists would cause performance problems (the problem states that the sequences could be large). My second attempt was very similar but passed in a StringBuilder. While this was more efficient it was less clear. My third attempt was to try and find a happy middle ground between the two. I ended up using recursion to generate a new IEnumerable which I could then consume and use a StringBuilder to combine. using System; using System.Text; using System.Linq; using System.Collections.Generic; public class CommaQuibble { private static IEnumerable<string> ToList(string[] strings, int start) { int length = strings.Length - start; if (length == 0) { yield return string.Empty; } else if (length == 1) { yield return strings[start]; } else if (length == 2) { yield return strings[start] + " and " + strings[start + 1]; } else { yield return strings[start] + ", "; /* Concatinate the result of the recursive call to the end of this IEnumerable. */ foreach(string s in ToList(strings, start + 1)) { yield return s; } } } public static string ToList(IEnumerable<string> strings) { StringBuilder stringBuilder = new StringBuilder("{"); foreach(string s in ToList(strings.ToArray(), 0)) { stringBuilder.Append(s); } stringBuilder.Append("}"); return stringBuilder.ToString(); } public static void Main() { List<string> list = new List<string>() {"one", "two", "three"}; Console.WriteLine(ToList(list)); } }Anonymous
April 16, 2009
I dont't like code that has to undo what proviously done before. But my standard solution for the original problem is to always append comma-value and than returning the final string from the second char on. For this problem I found a nice solution with very few variables, and no backtracking: string Stringize1(IEnumerable<string> list) { StringBuilder sb = new StringBuilder("{"); IEnumerator<string> enumerator = list.GetEnumerator(); if(enumerator.MoveNext()) { sb.Append(enumerator.Current); if(enumerator.MoveNext()) { string current = enumerator.Current; while(enumerator.MoveNext()) { sb.AppendFormat(", {0}",current); current = enumerator.Current; } sb.AppendFormat(" and {0}",current); } } return sb.Append("}").ToString(); } The inner declaration of the string variable and the while-loop can be also expressed this way: string current; for(current=enumerator.Current; enumerator.MoveNext(); current=enumerator.Current) sb.AppendFormat(", {0}",current); but this is a matter of taste... I prefer the former, because I need to explicitly define the string var outside the loop, because I need to use it after the loop termination!Anonymous
April 16, 2009
Olivier Leclant's answer is by far the best.Anonymous
April 16, 2009
Here's the smallest, efficient solution. Stringbuilder, and track the position of the last comma, for conversion to " and ". static string PrettyPrint(IEnumerable<string> strings) { bool firstString = true; int lastCommaPos = -1; /* no comma, yet / var sb = new StringBuilder(); sb.Append("{"); foreach (var s in strings) { if (!firstString) { lastCommaPos = sb.Length; sb.Append(", "); } sb.Append(s); firstString = false; } / if we have a final comma, turn it into " and " */ if (lastCommaPos != -1) sb.Replace(", ", " and ", lastCommaPos, 2); sb.Append("}"); return sb.ToString(); }Anonymous
April 16, 2009
I like Jafar Husain's strategy and have attempted a port to Ruby. There are pattern matching implementations for Ruby, but nothing in the core language, so here a case statement has to suffice. module Enumerable def bracketed_english_join s = "{" triple { |one, two, three| case when two == nil s << "" when one == nil && three == nil s << two when one == nil s << two when three == nil s << " and " << two else s << ", " << two end } s << "}" end def triple prepre = pre = current = nil; yield [nil, nil, nil] each do |item| prepre = pre; pre = current; current = item; yield [prepre, pre, current] end yield [pre, current, nil] end end if FILE == $0 require 'test/unit' class BracketedEnglishJoinTestCase < Test::Unit::TestCase def test_empty_returns_empty_string assert_equal('{}', [].bracketed_english_join) end def test_single_returns_item_only assert_equal( '{ABC}', ['ABC'].bracketed_english_join ) end def test_dual_returns_and_separated assert_equal( '{ABC and DEF}', ['ABC', 'DEF'].bracketed_english_join ) end def test_many_returns_comma_then_and_separated assert_equal( '{ABC, DEF, G and H}', ['ABC', 'DEF', 'G', 'H'].bracketed_english_join ) end end endAnonymous
April 16, 2009
@rbirkby: Could you point out a usage of StringBuilder in this thread which is inappropriate? Yes, the compiler will use String.Concat - but in this situation you really do want to use StringBuilder.Anonymous
April 16, 2009
Just to clarify my last comment - it's fair to say that if you're already using string.Join (as rbirkby's solution does) then using StringBuilder wouldn't help much. However, the solutions which only build the result up as they go without any duplicate strings being created (beyond StringBuilder buffer doubling) are going to be better off using StringBuilder than string concatenation.Anonymous
April 16, 2009
Not so efficient since I cannot assume the IEnumerable be a specific containar and need to count elementes, but to me simple enough. public string StringCommas(IEnumerable<string> collection) { // Count Elements int numberOfCollection = 0; foreach (string item in collection) { numberOfCollection++; } int numOfLeftSeparator = numberOfCollection - 1; string result = ""; foreach (string item in collection) { result += item; if (numOfLeftSeparator == 1) { result += " and "; } else if (numOfLeftSeparator > 1) { result += " ,"; } numOfLeftSeparator--; } return "{" + result + "}"; }Anonymous
April 16, 2009
I have extract position detection algorythm from my previous post to extension method, which seems to be very usable in similar scenarious. public static string Join2(IEnumerable<string> strings) { var delimiter = new Dictionary<ItemPosition, string> { {ItemPosition.First, ""}, {ItemPosition.Single, ""}, {ItemPosition.Default, ", "}, {ItemPosition.Last, " and "}, }; return strings .GetPositions() .Aggregate( new StringBuilder("{"), (sb, item)=> sb .Append(delimiter[item.Position]) .Append(item.Value), sb => sb .Append("}") .ToString() ); } [Flags] public enum ItemPosition { Default = 0, First = 1, Last = 2, Single = First | Last, } public class PositionedItem<T> { private ItemPosition m_position; private T m_value; public PositionedItem(ItemPosition position, T value) { m_position = position; m_value = value; } public ItemPosition Position { get { return m_position; } } public T Value { get { return m_value; } } } public static IEnumerable<PositionedItem<T>> GetPositions<T>(this IEnumerable<T> items) { T current = default(T); bool thereAreItems = false; ItemPosition position = ItemPosition.First; foreach (var item in items) { if (thereAreItems) { yield return new PositionedItem<T>(position, current); position = ItemPosition.Default; } current = item; thereAreItems = true; } if (!thereAreItems) { yield break; } position |= ItemPosition.Last; yield return new PositionedItem<T>(position, current); } (It seems my post is not appears in a couple of hours, so I repost it with little changes.)Anonymous
April 16, 2009
It seems to me that everyone using the Count() extension method on IEnumerable multiple times would do well to just translate the IEnumerable<string> to a List<string>, as each invocation of Enumerable.Count() does a complete iteration. The List implementation keeps that count internally, making it a single operation to retrieve it. You'll take a single hit converting it to a List<>, as opposed to multiple hits calling Count() multiple times.Anonymous
April 16, 2009
Sorry. A little bit better removng an else :) public string StringCommas(IEnumerable<string> collection) { // Count Elements int numberOfCollection = 0; foreach (string item in collection) { numberOfCollection++; } int numOfLeftSeparator = numberOfCollection - 1; string result = ""; foreach (string item in collection) { result += item; if (numOfLeftSeparator == 1) { result += " and "; } if (numOfLeftSeparator > 1) { result += " ,"; } numOfLeftSeparator--; } return "{" + result + "}";Anonymous
April 16, 2009
This is my two cents :-) using System;
using System.Collections.Generic;
namespace lippert_strings
{
class MainClass
{
public static void Main(string[] args)
{
Console.WriteLine("{}" == SmartJoin(new string[0]));
Console.WriteLine("{ABC}" == SmartJoin(new string[]{"ABC"}));
Console.WriteLine("{ABC and DEF}" == SmartJoin(new string[]{"ABC", "DEF"}));
Console.WriteLine("{ABC, DEF, G and H}" == SmartJoin(new string[]{"ABC", "DEF", "G", "H"}));
Console.WriteLine("{ABC, "DE, F", G and H}" == SmartJoin(new string[]{"ABC", "DE, F", "G", "H"}));
Console.WriteLine("{"A and BC", DEF, G and H}" == SmartJoin(new string[]{"A and BC", "DEF", "G", "H"}));
Console.ReadKey();
} private static string EscapeString(string source)
{
if(source.IndexOf(' ') != -1 ||
source.IndexOf(',') != -1 ||
source.IndexOf('{') != -1 ||
source.IndexOf('}') != -1)
return """ + source + """;
return source;
} public static string SmartJoin(IEnumerable<string> source)
{
string lastValue = null;
List<string> firstValues = new List<string>();
foreach(var current in source)
{
if(lastValue != null)
firstValues.Add(lastValue);
lastValue = EscapeString(current);
}
var firstValuesStr = "";
if(firstValues.Count > 0)
{
firstValuesStr = string.Join(", ", firstValues.ToArray()) + " and ";
}
var result = string.Format("{{{0}{1}}}", firstValuesStr, lastValue);
return result;
}
}
}Anonymous
April 16, 2009
Sam the Count() extension does a sneak peak at the type and the standard collection implementations will result in a call to Count property rather than enumerationAnonymous
April 16, 2009
Fun problem! Elegance is in the eye of the beholder, but this one's at least a little different from those posted by others. Single pass, no back-patching of the output, and fairly readable: static string InsertCommas(IEnumerable<string> strings)
{
StringBuilder builder = new StringBuilder();
bool first = true;
builder.Append('{');
strings.Aggregate(
(string)null, //init prev to null
(prev, current) =>
{
if (prev != null)
{
if (!first)
builder.Append(", ");
first = false;
builder.Append(prev);
}
return current;
},
(last) =>
{
if (last != null)
{
if (!first)
builder.Append(" and ");
builder.Append(last);
}
return string.Empty;
});
builder.Append('}');
return builder.ToString();
}Anonymous
April 16, 2009
I didn't add StringBuilder, to keep the posting short, but here is mine. I used regex instead of complicated logic "remembering" the last type, etc. using System;
using System.Text.RegularExpressions;
namespace CommaQuibbling
{
class Program
{
static void Main(string[] args)
{
// Output:
// {}
// {ABC}
// {ABC and DEF}
// {ABC, DEF, G and H}
Console.WriteLine(joinWords(""));
Console.WriteLine(joinWords("ABC"));
Console.WriteLine(joinWords("ABC", "DEF"));
Console.WriteLine(joinWords("ABC", "DEF", "G", "H"));
Console.ReadLine();
}
private static string joinWords(params string[] words)
{
// Add the commas and brackets
string returnString = "{" + string.Join(", ", words) + "}";
// Add the "and", and remove the last comma
string pattern = @"^(?<First>.*), (?<Last>.*)$";
returnString = Regex.Replace(returnString, pattern,
match => match.Groups["First"].Value + " and " + match.Groups["Last"].Value);
return returnString;
}
}
}Anonymous
April 16, 2009
This is my attempt at making the code quite explicit in what it's doing by using extension methods. It's not the most efficient of ways but I think it does say what it trying to do (arguably anyway). //the main method
public string StringQuibble( IEnumerable<string> strings )
{
return "{" + strings.Concat( ", ").ReplaceLastDelimiter( ", ", " and " ) + "}";
}
///the extension methods
public static class StringQubbleExtensions
{
public static string Concat( this IEnumerable<string> strings, string delimiter )
{
StringBuilder sb = new StringBuilder();
if( strings != null )
{
IEnumerator<string> stingsEnumerator = strings.GetEnumerator();
if( stringsEnumerator.MoveNext() )
{
sb.Append( stringsEnumerator.Current );
while( stringsEnumerator.MoveNext() )
{
sb.Append( delimiter );
sb.Append( stringsEnumerator.Current );
}
}
}
return sb.ToString();
} public static string ReplaceLastDelimiter( this string str, string delimiterToReplace, string delimiterReplacement )
{
if( str.IndexOf( delimiterToReplace ) > -1 )
{
StringBuilder sb = new StringBuilder( str );
return sb.Replace( delimiterToReplace, delimiterReplacement, str.LastIndexOf( delimiterToReplace), delimiterToReplace.Length ).ToString();
}
return str;
}
}Anonymous
April 16, 2009
The comment has been removedAnonymous
April 16, 2009
public static string FormatList(IEnumerable<string> source)
{
string last = source.DefaultIfEmpty().Last();
return "{" +
source.DefaultIfEmpty()
.Aggregate(new StringBuilder(),
(acc, next) => acc.AppendFormat(", {0}", next),
acc => Regex.Replace(acc.ToString().Substring(2), ", " + last + "$", " and " last))
+ "}";
}Anonymous
April 16, 2009
@Chris Benard: what if the last string contains " ,"?Anonymous
April 16, 2009
The comment has been removedAnonymous
April 16, 2009
Revised version which only iterates the source once: public static string FormatList(IEnumerable<string> source) { string last = string.Empty; return "{" + source.DefaultIfEmpty() .Aggregate(new StringBuilder(), (acc, next) => acc.AppendFormat(", {0}", last = next), acc => Regex.Replace(acc.ToString().Substring(2), ", " + last + "$", " and " + last)) + "}"; }Anonymous
April 16, 2009
Hideously obfuscated solution: public static string FormatList(IEnumerable<string> source) { var array = source.ToArray(); return "{" + ((array.Length == 0) ? string.Empty : (array.Length == 1) ? array[0] : string.Join(", ", array, 0, array.Length - 1) + " and " + array[array.Length - 1]) + "}"; }Anonymous
April 16, 2009
Eric, I like your use of Aggregate().Anonymous
April 16, 2009
It is not so costly to materialize list of strings in this task. Resulting string will consume same amount of memory if average length of word will be two characters (on x86). So we can simply get list and look on indexies. public static string Join3(IEnumerable<string> strings) { var list = (strings as IList<string>) ?? strings.ToList(); var result = new StringBuilder("{"); for (int i = 0; i < list.Count; i++) { result .Append(i == 0 ? "" : (i == list.Count - 1) ? " and " : ", ") .Append(list[i]); } return result .Append("}") .ToString(); } Or do little more optomization in case when source is ICollection. public static string Join4(IEnumerable<string> strings) { var list = (strings as ICollection<string>) ?? strings.ToList(); var result = new StringBuilder("{"); int i = 0; foreach (var item in list) { result .Append(i == 0 ? "" : (i == list.Count - 1) ? " and " : ", ") .Append(item); i++; } return result .Append("}") .ToString(); }Anonymous
April 16, 2009
The comment has been removedAnonymous
April 16, 2009
The comment has been removedAnonymous
April 16, 2009
@rbirkby: Of those, only Pankaj Sharma's solution seems to be obviously convertible to use string concatenation with no loss of either readability or performance. Yes, some of those others could be converted into one very complicated string concatenation, possibly including conditional expressions - but I don't think that's actually a good idea.Anonymous
April 16, 2009
To make it clean for all who uses Count(). Lets assume this simple test: static void Main(string[] args) { string result; var start = DateTime.Now; { result = Join(GetStrings(10)); } var end = DateTime.Now; Console.WriteLine(@"It takes {0}s to get ""{1}"" result", (end-start).TotalSeconds, result); Console.WriteLine("Press enter..."); Console.ReadLine(); } public static IEnumerable<string> GetStrings(int n) { for (int i = 0; i < n; i++) { // Do some hard work here... System.Threading.Thread.Sleep(500); yield return ((char)('a' + i)).ToString(); } } There source does not implement anything except IEnumerable and enumeration is very costly. So usualy it effectively to get list first of all and then manipulate on it rather than call Count() which internally performs enumeration and then enumerate twice.Anonymous
April 16, 2009
The comment has been removedAnonymous
April 16, 2009
Here is my proposal:
using System; using System.Collections.Generic; using System.ComponentModel; using System.Data; using System.Drawing; using System.Linq; using System.Text; using System.Windows.Forms; namespace LippertChallange0415 { public partial class Form1 : Form { public Form1() { InitializeComponent(); this.Load += new EventHandler(Form_Load); } public void Form_Load(object sender, EventArgs e) { StringBuilder collector = new StringBuilder(); collector.Append(GetCommaQuibbledString(new string[] {})); collector.Append("n"); collector.Append(GetCommaQuibbledString(new string[] {null, "NICK", "IS", "GOOD LOOKING", "COOL"})); collector.Append("n"); collector.Append(GetCommaQuibbledString(new string[] {"PEANUT BUTTER", "JELLY" })); collector.Append("n"); collector.Append(GetCommaQuibbledString(new string[] {"1", "2", "3", "4", "5", "6", "7", "8", "9", "10" })); collector.Append("n"); MessageBox.Show(collector.ToString()); } public string GetCommaQuibbledString(IEnumerable<string> input) { StringBuilder rtrn = new StringBuilder(); rtrn.Append("{"); if (input != null) { IEnumerator<string> strings = input.GetEnumerator(); if (strings.MoveNext()) { string current = null; string next = strings.Current ?? string.Empty; int addedCount = 0; // keep going as long as we haven't processed the last item while (next != null) { // replace current with next, get the next item if (strings.MoveNext()) { current = next; next = strings.Current ?? string.Empty; // add the appropriate separator (if any) if (addedCount > 0) rtrn.Append(", "); } else { current = next; next = null; // add the appropriate separator (if any) if (addedCount > 0) rtrn.Append(" and "); } // add the current item rtrn.Append(current); addedCount++; } } } rtrn.Append("}"); return rtrn.ToString(); } } }
Anonymous
April 16, 2009
Wow, all these insane complicated and long-winded solutions. I hope some people are just trying to be funny. I would use the exact same solution I used before with a small wrinkle: StringBuilder sb = new StringBuilder("{"); string token = null; foreach (B item in items) { if (sb.Length > 1) { sb.Append(", "); } sb.Append(token); token = item.SomeProperty; } if (!string.IsNullOrEmpty(token)) { if ((sb.Length > 1) && !string.IsNullOrEmpty(token)) { sb.Append(" and "); } sb.Append(token); } sb.Append("}"); That's it. No counters, no raw enumeration, no list conversion, no insanity. LINQ is great but I don't think it's appropriate for the problem at hand, performance-wise.Anonymous
April 16, 2009
//------------------------------------------------------------------ static string Quiz(IEnumerable<string> items) { var builder = new StringBuilder(); int counter = 0; string prev = null; foreach (var str in items) { if (counter++ == 0) { builder.Append(str); } else { if (prev != null) { builder.Append(", "); builder.Append(prev); } prev = str; } } if (counter > 1) { builder.Append(" and "); builder.Append(prev); } return String.Concat("{", builder.ToString(), "}"); }Anonymous
April 16, 2009
static string ConcatStrings(IEnumerable<string> strings) { int i = 0, c = strings.Count(); return strings.Aggregate(new StringBuilder("{"), (b, s) => i++ == 0 ? b.Append(s) : b.Append(i < c ? ", " : " and ").Append(s), b => b.Append("}").ToString()); }Anonymous
April 16, 2009
After implmenting the solution I checked and Yoav Zobel Had the one closet to mine (he chose to check for first rather than clean up aftewards) static string Quibble(IEnumerable<string> words) { StringBuilder builder = new StringBuilder(); int lastCommaPosition = 0 ; int penultimateCommaPosition = 0; foreach (string word in words) { builder.Append(word); penultimateCommaPosition = lastCommaPosition; lastCommaPosition = builder.Length; builder.Append(", "); } //Remove the last comma; if (lastCommaPosition > 0) builder.Remove(lastCommaPosition , 2); //Replace the penultimateComma with an and if (penultimateCommaPosition > 0) builder.Replace(",", " AND", penultimateCommaPosition, 1); return "{" + builder.ToString() + "}"; }Anonymous
April 16, 2009
This one is in Java (but the algorithm is what's important, and the code should be readable to any programmer): /** * This solves the problem described in * <a href="http://blogs.msdn.com/ericlippert/archive/2009/04/15/comma-quibbling.aspx">Eric * Lippert's Blog</a>. It performs in O(Nlog(N)) by keeping the collected items with * commas in a StringBuilder (which performs in O(Nlog(N)) by maintaining a character array * and re-sizing it on demand to a multiple of its current size) and keeping the last item in * a separate variable and concatenating it at the end. * * @param strings an iterable collection of non-null Strings. We are only allowed to iterate this * collection; no other operations are permitted. * @return a string which meets the requirements specified in the problem. */ public static String solution(Iterable<String> strings) { StringBuilder commaSeparated = null; String lastItem = null; for (String s : strings) { assert s != null; // the contract guaranteed elements would be non-null if (lastItem != null) { if (commaSeparated == null) { // First thing to go in the buffer goes on its own commaSeparated = new StringBuilder(lastItem); } else { // Subsequent things are comma-separated commaSeparated.append(", "); commaSeparated.append(lastItem); } } lastItem = s; } if (lastItem == null) { // There were 0 items in the collection return "{}"; } else { if (commaSeparated == null) { // There was 1 item in the collection return "{" + lastItem + "}"; } else { // There was more than 1 item in the collection return "{" + commaSeparated + " and " + lastItem + "}"; } } }Anonymous
April 16, 2009
static string EnglishJoin(IEnumerable<string> ls) { var result = new StringBuilder("{"); string last = null; int count = 0; foreach(var cur in ls) { count++; if(count > 2) result.Append(", "); if(count > 1) result.Append(last); last = cur; } if(count > 1) result.Append(" and "); result.Append(last); result.Append("}"); return result.ToString(); }Anonymous
April 16, 2009
I was looking for a mix between code maintainability and performance, so I tried the following approaches:
- StupidJoin: create a List from the Enumerable, use a for().
- StupidJoinOptimized: count the items of the Enumerable, then use foreach() and an index
- NaiveJoin: build the string with all "," separators, remember the index of the last one; then, replace the last with " and"
- GetCombined: Rick Dailey method
- GetCombinedOptimized: Rick Dailey method, with a "first" boolean, to avout calculating builder.Length at each loop All methods use a StringBuilder. For a small array, repeated many times, I got my expected results (small increase with each optimization): ================================================================ 40 ARRAY ITEMS, 3000000 TIMES ================================================================ 3000000 StupidJoin done in 22143 ms ================================================================ 3000000 StupidJoinOptimized done in 21115 ms (5% time saved) SMALL SAVE ================================================================ NaiveJoin benchmark 3000000 NaiveJoin done in 18427 ms (17% time saved) MY IDEA, NICE SAVE ================================================================ 3000000 GetCombined done in 14874 ms (33% time saved) YEP, RICK DAILEY IS SMARTER... ================================================================ 3000000 GetCombinedOptimized done in 14484 ms (35% time saved) ...BUT CAN BE OPTIMIZED Then I tried a very long array, looped a few times, and the results were different! ================================================================ 4000 ARRAY ITEMS, 30000 TIMES ================================================================ 30000 StupidJoin done in 29132 ms ================================================================ 30000 StupidJoinOptimized done in 13430 ms (54% time saved) WTF! THE STUPID APPROACH CAN BE FASTER THAN RICK DAILEY GETCOMBINED ================================================================ 30000 NaiveJoin done in 16031 ms (45% time saved) ================================================================ 30000 GetCombined done in 13698 ms (53% time saved) ================================================================ 30000 GetCombinedOptimized done in 12938 ms (56% time saved) These calls were made without actually assigning the method result to a variable. Assigning the result to a variable, actually gave totally different results (still have to dig into that). If you're interested, here is the code of the methods: private static string StupidJoin(IEnumerable<string> strings) { List<string> list = new List<string>(strings); int total = list.Count; StringBuilder sb = new StringBuilder(); sb.Append("{"); for (int n = 0; n < total; n++) { if (n == total - 1) sb.Append(" and "); else if(n > 0 ) sb.Append(", "); sb.Append(list[n]); } sb.Append("}"); return sb.ToString(); } private static string StupidJoinOptimized(IEnumerable<string> strings) { StringBuilder sb = new StringBuilder(); sb.Append("{"); int total = strings.Count(); int n = 0; foreach (string s in strings) { if (n == total - 1) sb.Append(" and "); else if (n > 0) sb.Append(", "); sb.Append(s); n++; } sb.Append("}"); return sb.ToString(); } private static string NaiveJoin(IEnumerable<string> strings) { StringBuilder sb = new StringBuilder(); sb.Append("{"); bool first = true; int lastComma = 0; int lastLength = 0; foreach (string s in strings) { lastLength = s.Length; if(!first) { sb.Append(", "); lastComma += lastLength + 2; } sb.Append(s); first = false; } if (lastComma > 0) lastComma--; sb.Append("}"); if (lastComma == 0) return sb.ToString(); return sb.ToString().Remove(lastComma, 1).Insert(lastComma, " and"); } static string GetCombined(IEnumerable<string> strings) { string opening = "{"; var builder = new StringBuilder(opening); var enumerator = strings.GetEnumerator(); bool hasNext = enumerator.MoveNext(); while (hasNext) { string s = enumerator.Current; hasNext = enumerator.MoveNext(); if (builder.Length > opening.Length) // after the opening curly brace builder.Append(hasNext ? ", " : " and "); builder.Append(s); } builder.Append('}'); return builder.ToString(); } static string GetCombinedOptimized(IEnumerable<string> strings) { string opening = "{"; var builder = new StringBuilder(opening); var enumerator = strings.GetEnumerator(); bool hasNext = enumerator.MoveNext(); bool first = true; while (hasNext) { string s = enumerator.Current; hasNext = enumerator.MoveNext(); if (!first) // after the opening curly brace builder.Append(hasNext ? ", " : " and "); builder.Append(s); first = false; } builder.Append('}'); return builder.ToString(); }
Anonymous
April 16, 2009
@Aaron G - cocky, or cockup? did you test it? :)Anonymous
April 16, 2009
@Filini - glad to see someone checking perf :) I think your GetCombinedOptimized is the same as my solution? But much prettier!Anonymous
April 16, 2009
@Aaron G: It's clearly a personal matter - I found your solution somewhat harder to understand than many of the others, due to always using the token in the iteration after it's retrieved, so on the first iteration you end up with sb.Append(null) - and mentally checking that yes, this does do nothing.Anonymous
April 16, 2009
oops. submitted too fast. @Filini - StupidJoinOptimized might be fast in your tests because by passing an array you gave it something with an optimized ".Count()" method. Try the tests with something that actually has to calculate & yield each word...Anonymous
April 16, 2009
The comment has been removedAnonymous
April 16, 2009
The comment has been removedAnonymous
April 16, 2009
After reading some of the answers I got an idea to add item to the list only after separator to be used is known. And to do that by delayed evaluation. Since I thought it would be a great exercise for me to learn a bit about delegate magic, I went implemented my idea. using System; using System.Collections.Generic; using System.Text; using System; using System.Collections.Generic; using System.Text; namespace NoOxfordComma { class Program { static string NoOxfordComma(IEnumerable<string> strings) { StringBuilder englishList = null; Action<string, string> addToList = (separator, item) => englishList = englishList == null ? new StringBuilder().Append(item) : englishList.Append(separator).Append(item); Func<string, Action<string>> addItem = (item) => (separator) => addToList(separator, item); Action<string> addSeparator = (s) => { }; foreach (string s in strings) { addSeparator(", "); addSeparator = addItem(s); } addSeparator(" and "); return "{" + englishList + "}"; } static void Main(string[] args) { Console.WriteLine(NoOxfordComma(new string[] { })); Console.WriteLine(NoOxfordComma(new string[] { "ABC" })); Console.WriteLine(NoOxfordComma(new string[] { "ABC", "DEF" })); Console.WriteLine(NoOxfordComma(new string[] { "ABC", "DEF", "G", "H" })); Console.WriteLine(NoOxfordComma(new string[] { "{", "", ",", "and", "}" })); Console.ReadKey(); } } }Anonymous
April 16, 2009
Here is my quick effort, before I posted I had a quick look at the other offerings and not surprisingly has more or less been posted already. string lastWord = string.Empty; StringBuilder result = new StringBuilder("{"); foreach (string word in words) { if(result.Length!=1) result.Append(","); result.Append(lastWord); lastWord = word; } if (result.Length != 1) result.Append(" and "); result.Append(lastWord); result.Append("}");Anonymous
April 16, 2009
The comment has been removedAnonymous
April 16, 2009
Sam... actually I was just saying Count() is cheaper regardless of the underlying class behind the enumerable. Counting once, building a new list of most items once, all adds up to an efficient solution... in my opinion. As far as terseness... I just like terseness, but I don't assume that greater terseness == greater readability for everybody. Filini... that looks pretty thorough. Can you include some variations that don't use StringBuilder? Specifically I mean options that use string.Join(...). I'm just curious.Anonymous
April 16, 2009
static string JoinStrings(IEnumerable<string> strings) { int len = strings.Count(); return len == 0 ? "{}" : len == 1 ? "{"+strings.First()+"}" : "{"+strings.Take(len - 1).Aggregate((string head, string tail) => head+", "+tail)+" and " +strings.Last()+"}"; }Anonymous
April 16, 2009
Hace unos dias Eric publico un problema , al principio me dio flojera responderlo, pero al ver el numeroAnonymous
April 16, 2009
The comment has been removedAnonymous
April 16, 2009
/// <summary> /// Solve comma quibbling posed by Eric Lippert at: /// http://blogs.msdn.com/ericlippert/archive/2009/04/15/comma-quibbling.aspx. /// </summary> /// <typeparam name="T">Type of input.</typeparam> /// <param name="input">Stream of input elements (ex: string).</param> /// <returns>Quibbled string.</returns> /// <remarks> /// Good points: /// * Generic type support (StringBuilder formats for output) /// * Special-case logic is not run every time through the loop /// Bad points: /// * Input stream is traversed three times :( /// </remarks> private static string CommaQuibbling<T>(IEnumerable<T> input) { // Capture stream var a = input.GetEnumerator(); var b = input.Skip(1).GetEnumerator(); var c = input.Skip(2).GetEnumerator(); // Prefix the result var sb = new StringBuilder("{"); // Process the "normal" leading elements while (c.MoveNext() && b.MoveNext() && a.MoveNext()) { sb.Append(a.Current).Append(", "); } // Process the non-Oxford comma scenario if (b.MoveNext() && a.MoveNext()) { sb.Append(a.Current).Append(" and "); } // Process the remaining element if (a.MoveNext()) { sb.Append(a.Current); } // Postfix the result and return it return sb.Append("}").ToString(); }Anonymous
April 16, 2009
ptoniolo... you're right, of course. Eric didn't mention any performance requirements, however, so I'm inclined to go with whatever is the most clear and elegant. But, with that being said, my earlier posts with Count() and ToArray()'s would be dreadfully slow. But this would be faster: string CommaQuibble(IEnumerable<string> words) { StringBuilder sb = new StringBuilder(100); string lastWord = null; int count = 0; sb.Append("{"); foreach (string word in words) { if (count > 1) sb.Append(", "); if (lastWord != null) sb.Append(lastWord); lastWord = word; count++; } if (count > 1) sb.Append(" and "); if (lastWord != null) sb.Append(lastWord); sb.Append("}"); return sb.ToString(); }Anonymous
April 16, 2009
Delay, It took a while for me to understand that you rely on the short-circuiting of the "&&" operator. I am sure the triple pass over the input stream is awful, but the solution is somewhat nice, amusing!Anonymous
April 16, 2009
Jafar was teasing me about his F# solution to Eric Lippert's comma quibbling problem , and I decidedAnonymous
April 16, 2009
The comment has been removedAnonymous
April 16, 2009
Oops. I have posted my solution a while ago, but then realised most of the others are trying to tackle the problem of the strings containing commas and curly brackets. I did notice the mentioning that the strins MAY contain anything, including commas and brackets, but nothing tells me what the resulting string should look like, if they do; so my solution just does not bother checking -- too bad for me... :-) Then again: I just keep remembering that pop culture expression, about whose mother assumption is, and try not to assume anything, unless I have to.Anonymous
April 16, 2009
This was a lot of fun, by the way. I love learning new ways to do things and how to do them more efficiently. I hope you enjoy my solution. The first portion of this is a method to parse the IEnumerable<string> parameter sent in. I did not add any comments here as I love comments and I wanted to just show the main functionality first to see if it would be considered readable without the comments. I will post the portion with comments below it. public static string ParseStrings(IEnumerable<string> strings) { string begin = "{"; string end = "}"; string body = String.Empty; if (strings.Count() > 1) { begin += strings.First(); end = " and " + strings.Last() + end; if (strings.Count() > 2) { foreach (string item in strings) { if (item != strings.First() && item != strings.Last()) body += "," + item; } } } else body += strings.SingleOrDefault<string>(); return begin + body + end; } This second portion is the entire class. I like playing with new things as I said, so I created a C# extension called CreateDelimitedList that will convert an IEnumerable<string> list to this new format. Then I created a few lists to test against. I ran them first through a method that returns the result of the extension, and then one that runs it directly through the extension. Even though I used List<string> in the method where this is being called, I made sure that the parameter that the actual code was being parsed against was IEnumerable<string>. Here is the entire solution below (remember I love commenting). using System; using System.Collections.Generic; using System.Linq; using System.Text; namespace CommaQuibbling { class Program { static void Main(string[] args) { //Make some lists for testing purposes List<List<string>> parsedLists = new List<List<string>>() { new List<string>(), new List<string>() {"a"}, new List<string>() { "a", "b" }, new List<string>() { "a", "b", "c" }, new List<string>() { "a", "b", "c", "d"}, new List<string>() { "a", "b", "c", "d", "e" } }; //parse each list by method Console.WriteLine("Parse by method"); foreach (List<string> list in parsedLists) { Console.WriteLine(ParseStrings(list)); } //parse each list by extension Console.WriteLine("Parse by extension"); foreach (List<string> list in parsedLists) { Console.WriteLine(list.CreateDelimitedList()); } Console.ReadKey(); } public static string ParseStrings(IEnumerable<string> strings) { //use the extension here -- the code in the extension could be moved here //to perform the same thing return strings.CreateDelimitedList(); } } public static class IEnumerableExtensions { public static string CreateDelimitedList(this IEnumerable<string> strings) { //create the begin and end string which will aways be the opening and closing bracket string begin = "{"; string end = "}"; //this will contain the middle portion of the string string body = String.Empty; //if the string count is great than 1 //if it is the only item add it to body //if there are no strings, it will bring back an empty string if (strings.Count() > 1) { //get the first string and the last string //putting the and before the last string item will ensure the last comma //wont become an issue because you can then put the comma before the 'body' item begin += strings.First(); end = " and " + strings.Last() + end; if (strings.Count() > 2) { foreach (string item in strings) { //if it is not the first or last item add the item in with a comma before it if (item != strings.First() && item != strings.Last()) body += "," + item; } } } else body += strings.SingleOrDefault<string>(); return begin + body + end; } } }Anonymous
April 16, 2009
I'm curious now... How are some people getting their code to be displayed without double newlines everywhere? Most seem to be formatted badly, but there are several by different people that look right. I'm fixing them manually with my comment editing tools. Apparently I'm drinking from a fire hose of solutions here. It'll take a while to get to them all. This has turned out to be a more popular pasttime than I anticipated! -- EricAnonymous
April 16, 2009
le sigh I realized the first portion of code lost its form (indentation and such) and that bothers me, but the premise is there. ;)Anonymous
April 16, 2009
Oh I also wanted to note that in my code I used the IEnumerable<T>.Count() which returns an int32, however if the list is going to be really long as was hinted at in the information above then there is a IEnumerable<T>.LongCount() that can be used which brings back an int64 instead. (See this article for more : http://msdn.microsoft.com/en-us/library/bb338038(VS.95).aspx)Anonymous
April 16, 2009
Of course the Stack-derived nonsense was completely nonsensical: using System.Collections.Generic; using System.Linq; using System.Text; using NUnit.Framework; namespace CommaQuibble { [TestFixture] public class QuibblerFixture { [Test] public void EmptyList() { IEnumerable<string> list = new[] {""}; string quibbled = GetQuibbled(list); Assert.AreEqual("{}", quibbled); } private static string GetQuibbled(IEnumerable<string> list) { Quibbler quibbler = new Quibbler(); return quibbler.Quibble(list); } [Test] public void OneItemInList() { IEnumerable<string> list = new[] {"ABC"}; string quibbled = GetQuibbled(list); Assert.AreEqual("{ABC}", quibbled); } [Test] public void TwoItemsInList() { string[] list = new[] { "ABC", "DE" }; string quibbled = GetQuibbled(list); Assert.AreEqual("{ABC and DE}", quibbled); } [Test] public void ThreeItemsInList() { string[] list = new[] { "ABC", "DE", "ZYXWV" }; string quibbled = GetQuibbled(list); Assert.AreEqual("{ABC, DE and ZYXWV}", quibbled); } [Test] public void ManyMany() { string[] list = new[] { "ABC", "DE", "ZYXWV", "FG", "UT", "HI", "SR", "JK", "QP", "LM", "NO" }; string quibbled = GetQuibbled(list); Assert.AreEqual("{ABC, DE, ZYXWV, FG, UT, HI, SR, JK, QP, LM and NO}", quibbled); } [Test] public void FarTooMany() { string[] list = new[] { "yhgv", "dsfgdsfgth", "adfg", "gtdfgt", "ag", "wrgsd", "gbtshgrty", "faevest", "htsbvtc", "hyryhcwtc", "cwtrchrsth", "qegsdf", "wruutyu", "w465tw", "cfgwdfg45", "fydw45fhw46", "wfywc546jh", "se5ys5ryhserh", "gvw46w6thdrtg", "ygsve54", "esy6", "5yfwcwerst", "rtugfwjuej", "sfv6jusrt", "s34etyva35y", "gyuiu87jdr", "gacegrasecgra", "e5h", "c5yw46h", "w5w46fyw45y", "wqf45ywe6fy", "fw45yw4y6w6u", "sw345f", "w45yf", "cvhwrthcwtr", "aery", "chwrthwrtc", "chq4t6h", "se4rtvcw5", "gcr", "cfw4gfw", "fgw456y", "df6w4", "dfqc4wh6f57fjhf", "q354fq54y" }; string quibbled = GetQuibbled(list); Assert.AreEqual("{yhgv, dsfgdsfgth, adfg, gtdfgt, ag, wrgsd, gbtshgrty, faevest, htsbvtc, hyryhcwtc, cwtrchrsth, qegsdf, wruutyu, w465tw, cfgwdfg45, fydw45fhw46, wfywc546jh, se5ys5ryhserh, gvw46w6thdrtg, ygsve54, esy6, 5yfwcwerst, rtugfwjuej, sfv6jusrt, s34etyva35y, gyuiu87jdr, gacegrasecgra, e5h, c5yw46h, w5w46fyw45y, wqf45ywe6fy, fw45yw4y6w6u, sw345f, w45yf, cvhwrthcwtr, aery, chwrthwrtc, chq4t6h, se4rtvcw5, gcr, cfw4gfw, fgw456y, df6w4, dfqc4wh6f57fjhf and q354fq54y}", quibbled); } [Test] public void ViacheslavIvanov() { string[] list = new[] { "", ",", "}" }; string quibbled = GetQuibbled(list); Assert.AreEqual("{, , and }}", quibbled); } } public class Quibbler { public string Quibble(IEnumerable<string> enumerable) { StringBuilder builder = new StringBuilder("{"); string last = enumerable.Last(); string first = enumerable.First(); if(first == last) { builder.Append(first); } else if(first != last) { IEnumerable<string> rest = enumerable.Except(new[] { last }); string penultimate = rest.Last(); foreach (string item in rest) { builder.Append(item); if (item != penultimate) builder.Append(", "); } builder.AppendFormat(" and {0}", last); } builder.Append("}"); return builder.ToString(); } } }Anonymous
April 16, 2009
Eric, you're really going to go through all of these? Do you sleep?Anonymous
April 16, 2009
Have I read all these - not a chance - so if you have read this far I hope this provides something extra :-). Too much free time people :-). class Program { static void Main(string[] args) { IEnumerable<string> items = null; items = new String[] { "A"}; PrintOutput(items); items = new String[] { "A", "B"}; PrintOutput(items); items = new String[] { "A", "B", "C"}; PrintOutput(items); items = new String[] { "A", "B", "C", "D", "X", "DEF" }; PrintOutput(items); Console.ReadLine(); } private static void PrintOutput(IEnumerable<string> items) { StringBuilder sb = new StringBuilder(60); //. Arbitrary value that would be determined based on best guess at most common volume. // 1. Always output initial curly brace. sb.Append("{"); IEnumerator<string> enumerator = items.GetEnumerator(); string[] buffer = new string[3]; int ordinal = 0; while (enumerator.MoveNext()) { buffer[ordinal] = enumerator.Current; ordinal++; if (ordinal == buffer.Length) { // Note: If I just output the first item then I know I have at least two more to output. sb.AppendFormat("{0}, ", buffer[0]); Array.ConstrainedCopy(buffer, 1, buffer, 0, 2); buffer[2] = null; ordinal--; } } if (ordinal == 1) // I have one to output { sb.Append(buffer[0]); } else if (ordinal == 2) // I have two to output { sb.AppendFormat("{0} and {1}", buffer[0], buffer[1]); } else // there be three { sb.AppendFormat("{0}, {1} and {2}", buffer[0], buffer[1], buffer[2]); } // Always output final curly brace. sb.Append("}"); // Wouldn't do this - provided for example review. Console.WriteLine(string.Format("Source data : {0}", string.Join(", ", items.ToArray()))); // Display result. Console.WriteLine(sb.ToString()); } }Anonymous
April 16, 2009
public static string ConcatString(IEnumerable<string> words) { string result = string.Empty; string[] temp = words.ToArray(); for (int i = temp.Length; i > 0; i--) { if (i == 1) result += temp[temp.Length - i]; else if (i == 2) result += temp[temp.Length - i] + " and "; else result += temp[temp.Length - i] + ", "; } return "{" + result + "}"; }Anonymous
April 16, 2009
Whoops needs to handle the first case. correction - else if // there be three should be else if (ordinal == 3)Anonymous
April 16, 2009
Well... public string GetConcatenated(IEnumerable<string> strings) { string result = ""; string[] list = strings.ToArray(); int length = list.Length; for(int i=0;i<length;i++) { if(result.Length > 0) if(i == length-1) result +=" and "; else result +=", "; result += list[i]; } return "{" + result + "}"; }Anonymous
April 16, 2009
namespace SampleApp { static class Program { static void Main(string[] args) { //string[] s = { "A", "B", "C and", "D", "", ",,,," }; //IEnumerable<string> str = s.ToArray().AsEnumerable<string>(); Console.WriteLine(BuildStringArray().Join(",", " and ")); } static IEnumerable<string> BuildStringArray() { string alphabets = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; // string[] elements = new string[100]; Random random = new Random(100); for (int i = 0; i < 10; i++) { int min = random.Next(0, 25); int max = random.Next(min, 25); yield return alphabets.Substring(min, max - min + 1); } } public static string Join(this IEnumerable<string> source, string delimeter1, string delimeter2) { int NumElem = source.Count(); StringBuilder finalString = new StringBuilder("{"); for (int i = 0; i < NumElem -1; i++) { finalString.Append(string.Format("{0}{1}", source.ElementAt<string>(i), (i + 1 < NumElem -1 ) ? delimeter1 : delimeter2)); } if (NumElem > 0) finalString.Append(source.ElementAt<string>(NumElem - 1)); finalString.Append("}"); return finalString.ToString(); } } }Anonymous
April 16, 2009
And without array and optimized iterator: public string GetConcatenated(IEnumerable<string> strings) { string result = ""; int maxIndex = strings.Count() - 1; for(int i=0;i<=maxIndex;i++) { if(result.Length > 0) if(i == maxIndex) result +=" and "; else result +=", "; result += strings.ElementAt(i); } return "{" + result + "}"; }Anonymous
April 16, 2009
My second attempt. This time with an eye for performance and readability instead of terseness. class Program { static void Main() { AreEqual(Stringify(new string[] {}), "{}"); AreEqual(Stringify(new [] {"ABC" }), "{ABC}"); AreEqual(Stringify(new[] { "ABC", "DEF" }), "{ABC and DEF}"); AreEqual(Stringify(new[] { "ABC", "DEF", "G", "H" }), "{ABC, DEF, G and H}"); } static string Stringify(IEnumerable<string> sequence) { var cons = new Cons<string>(sequence); var head = cons.Head().ToArray(); var and = head.Length > 0 ? " and " : string.Empty; return "{" + string.Join(", ", head) + and + cons.Tail + "}"; } static void AreEqual(string actual, string expected) { if (actual != expected) throw new Exception(actual + "!=" + expected); } } This iterates the sequence twice, once to generate the array and a second time to join with comma delimiters. However, this solution also needs a Cons class: class Cons<T> { private readonly IEnumerable<T> _sequence; private T _current; public Cons(IEnumerable<T> sequence) { _sequence = sequence; } public IEnumerable<T> Head() { IEnumerator<T> enumerator = _sequence.GetEnumerator(); if (enumerator.MoveNext()) { while (true) { _current = enumerator.Current; if (!enumerator.MoveNext()) { yield break; } yield return _current; } } } public T Tail { get { return _current; } } } Yes, the Cons class has some bad API design, but nothing that couldn't be solved without some extra logic.Anonymous
April 16, 2009
Here's mine: Please replace Words.txt with any file containing words(one word on each line). public static IEnumerable<string> GetWords() { using (StreamReader reader = new StreamReader("Words.txt")) { while(!reader.EndOfStream) { string word = reader.ReadLine(); yield return word; } yield break; } } public static string Concat() { string[] excludes = { ",", "{", "}", "" }; string str; IEnumerable<string> words = GetWords().Select(s => s.Trim()).Except(excludes); if(words.SequenceEqual(Enumerable.Empty<string>())) { str = "{}"; return str; } if(words.ElementAtOrDefault(0) != null && words.ElementAtOrDefault(1) == null) { str = words.SingleOrDefault(); return str; } if (words.ElementAtOrDefault(1) != null && words.ElementAtOrDefault(2) == null) { str = string.Concat("{", words.ElementAt(0), " and ", words.ElementAt(1), "}"); return str; } else { StringBuilder sb = new StringBuilder(); string s = words.Aggregate((w1, w2) => string.Concat(w1, ",", w2)); sb.Append(s); sb.Replace(",", " and " , s.LastIndexOfAny(new char[]{','}),1); str = sb.ToString(); return str; } }Anonymous
April 16, 2009
@Abhijeet For the input abc bca, cab} bca Output: abc,bca,,cab} and bca For the input: abc bca, cab} bca, Output: abc,bca, and cab} Is that what you are getting? Is this correct? @Eric, Could you please let me know if my program is correct? I am wondering whether I understood the problem or not. -Jagannath.Anonymous
April 16, 2009
I just went for an attempt on readability with no considerations in terms of performance. The solution is based on Jon Skeet approach. using System; using System.Collections.Generic; using System.Text; namespace CommaQuibbling { class Program { static void Main() { Test(); Test("ABC"); Test("ABC", "DEF"); Test("ABC", "DEF", "G", "H"); } static void Test(params string[] words) { Console.WriteLine(BuildWordList(words)); } static string BuildWordList(IEnumerable<string> words) { StringBuilder list = new StringBuilder(); list.Append("{"); // Iterate list indexing the last (2) elements foreach (Element<string> element in Element<string>.IterateListWithBackwardIndexing(words, 2)) { list.Append(element.Value); if (element.IsBackwardIndexed) { if (element.BackwardIndex == 1) { // Suffix for penultimate word list.Append(" and "); } } else { list.Append(", "); } } list.Append("}"); return list.ToString(); } } class Element<T> { private int backwardIndex; Element(T value) { this.Value = value; this.IsBackwardIndexed = false; } Element(T value, int backwardIndex) { this.Value = value; this.IsBackwardIndexed = true; this.BackwardIndex = backwardIndex; if (this.BackwardIndex == 0) { this.IsLast = true; } } public T Value { get; private set; } public bool IsLast { get; private set; } public bool IsBackwardIndexed { get; private set; } public int BackwardIndex { get { if (this.IsBackwardIndexed) { return this.backwardIndex; } throw new InvalidOperationException("The element is not backward indexed."); } private set { this.backwardIndex = value; } } public static IEnumerable<Element<T>> IterateListWithBackwardIndexing(IEnumerable<T> list, int elementsToIndex) { if (elementsToIndex < 1) { throw new ArgumentOutOfRangeException("elementsToIndex", "The number of elements to index must be greater than zero."); } Queue<T> buffer = new Queue<T>(elementsToIndex); foreach (T item in list) { if (buffer.Count == elementsToIndex) { // Return unindexed elements as soon as the buffer is filled yield return new Element<T>(buffer.Dequeue()); } buffer.Enqueue(item); } while (buffer.Count != 0) { // Return the last elements with associated backward index yield return new Element<T>(buffer.Dequeue(), buffer.Count); } } } }Anonymous
April 16, 2009
@SteveEisner: Yes, how fast the .Count() is performed is important, and the idea in my different tests was to only call it once (or never). But I didn't think that the .Count() could become a real bottleneck, I couldn't think of a real IEnumerator that performed supa-fast while iterating, but turtle-slow when calling the .Count(). @Joren: there wasn't a real "purpose" in my tests to improve arbitrary programs, I was just curious on this: "I'll try a stupid cow-like approach to solve the problem, then I try to improve my algorithm, and see if optimization matters". Then I thought: "are the algorithms posted by other people much faster than mine?" and I just made some tests. Finally, I saw how the length of the IEnumerable affects things, and I found it interesting, that's it :) @Todd: when I have some time, I'll test some other implementations posted here, with string.Join() and LINQ - even if I find it friggin' unreadable :P Overall, I just wanted to see how the Count() call and the IEnumerable length affect the different implementations. And I was really astonished how my semi-stupid approach performs quite fast for long enumerables.Anonymous
April 16, 2009
The comment has been removedAnonymous
April 16, 2009
@ Jafar PowerPack is a supported library one of the supported libraries FSharp.Core.dll FSharp.PowerPack.dll FSharp.PowerPack.Linq.dll http://blogs.msdn.com/dsyme/archive/2008/08/29/the-f-september-2008-ctp-is-now-available.aspx I think the declarative nature of F# is a big win for this problem (The 4 pattern matching cases reads just like the the 4 characteristics of the specification)
- seq for IEnumerable
- LazyList for pattern matching
- avoid stack overflow with tailrecursion Declarative solution without performance penalty Happy FSharp hacking
Anonymous
April 16, 2009
The comment has been removedAnonymous
April 16, 2009
Please find a small change in my code where I am using string array instead of IEnumerable in the Join function. This performs much better. using System; using System.Diagnostics; using System.Collections.Generic; using System.Collections; using System.Linq; using System.Text; namespace SampleApp { static class Program { static void Main(string[] args) { IEnumerable<string> elems = BuildStringArray(); System.Diagnostics.Stopwatch sw = new Stopwatch(); sw.Start(); //Console.WriteLine(elems.Join(",", " and ")); //elems.Join(",", " and "); Join(elems.ToArray<string>(), ",", " and "); TimeSpan ts = sw.Elapsed; //Console.WriteLine("{0}:{1}:{2}:{3}", ts.Hours, ts.Minutes, ts.Seconds, ts.Milliseconds); Console.WriteLine(ts.TotalSeconds); } static IEnumerable<string> BuildStringArray() { string alphabets = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; // string[] elements = new string[100]; Random random = new Random(40); for (int i = 0; i < 100000; i++) { int min = random.Next(10, 25); int max = random.Next(min, 25); yield return alphabets.Substring(min, max - min + 1); } } public static string Join(string[] source, string delimeter1, string delimeter2) { int NumElem = source.Count(); StringBuilder finalString = new StringBuilder("{"); for (int i = 0; i < NumElem -1; i++) { finalString.Append(string.Format("{0}{1}", source[i], (i + 1 < NumElem -1 ) ? delimeter1 : delimeter2)); } if (NumElem > 0) finalString.Append(source[NumElem - 1]); finalString.Append("}"); return finalString.ToString(); } } }Anonymous
April 17, 2009
@Mark Rendle: At one stage I had an AddDelimiters() method like yours - I liked the idea of inserting the delimiters into the sequence stream. However it eventually morphed into the Cons class I posted.Anonymous
April 17, 2009
Now I'm home and fed, I can do it the way I wanted originally - with the built in Queue<string>. Honestly, I started this way with my earlier post, but stepped back and coded a buffer explicitly - I shouldn't have bothered. This will work with a TB source (providing it's streamed via a file or IDataReader). Full Console Program. class Program { static void Main(string[] args) { List<string> source = new List<string>(new String[] { "A", "B", "C" }); // How to use a StringBuilder - StringBuilder stringBuilder = new StringBuilder(); StringWriter writer = new StringWriter(stringBuilder); WriteOutput(source, writer); Console.WriteLine("StringBuilder output."); Console.WriteLine(stringBuilder.ToString()); Console.WriteLine("--------------------"); Console.WriteLine(); // Do some heavy lifting // Creating a dummy IEnurable source - could be a ten GB file, db reader etc. Console.WriteLine("Do some heavier lifting - Streamed output."); Console.WriteLine("Press any key to continue."); Console.WriteLine(); Console.ReadLine(); source.Clear(); // Empty and repopulate source int maxItems = 100; // Try 0 and 1 and 100000. for (int i = 0; i < maxItems; i++) { source.Add(i.ToString()); } // Stream WriteOutput(source, Console.Out); Console.ReadLine(); } /// <param name="writer">TextWriter param supports streaming output</param> static void WriteOutput(IEnumerable<string> items, TextWriter writer) { Queue<string> buffer = new Queue<string>(3); writer.Write("{"); foreach (string item in items) { buffer.Enqueue(item); if (buffer.Count == 3) { writer.Write(String.Format("{0}, ", buffer.Dequeue())); } writer.Flush(); // Slow down for user viewing // Drop this. System.Threading.Thread.Sleep(50); } if (buffer.Count == 1) // I have one to output { writer.Write(buffer.Dequeue()); } else if (buffer.Count == 2) // I have two to output { writer.Write("{0} and {1}", buffer.Dequeue(), buffer.Dequeue()); } else if (buffer.Count == 3) { writer.Write("{0}, {1} and {2}", buffer.Dequeue(), buffer.Dequeue(), buffer.Dequeue()); } writer.Write("}"); } } PS: I Think I'm about to come up as mokeefe (Martin O'Keefe).Anonymous
April 17, 2009
Simple and Fast (I guess): using System; using System.Collections.Generic; using System.Linq; using System.Text; namespace CSharpTest { static class Solution { static StringBuilder Append(this StringBuilder sb, string one, string two) { return sb.Append(one).Append(two); } static string Next(this IEnumerator<string> enumerator) { enumerator.MoveNext(); return enumerator.Current; } public static string GetSequence(IEnumerable<string> items) { const string OPEN = "{", CLOSE = "}"; //(1) If the sequence is empty then the resulting string is "{}" if (null == items) return OPEN + CLOSE; int itemsCount = items.Count(); switch (itemsCount) { //(1) If the sequence is empty then the resulting string is "{}" case 0: return OPEN + CLOSE; //(2) If the sequence is a single item "ABC" then the resulting string is "{ABC}" case 1: return OPEN + items.First() + CLOSE; //(3) If the sequence is the two item sequence "ABC", "DEF" then the resulting string is "{ABC and DEF}". //(4) If the sequence has more than two items, say, "ABC", "DEF", "G", "H" then the resulting string is "{ABC, DEF, G and H}". (Note: no Oxford comma!) default: const int COUNTDOWN = 2, ESTIMATED_STRING_LENGTH = 4; const string SEPARATOR = ",", SEPARATOR_END = "and"; var sb = new StringBuilder(OPEN, ESTIMATED_STRING_LENGTH * itemsCount); var enumerator = items.GetEnumerator(); while (COUNTDOWN < itemsCount--) sb.Append(enumerator.Next(), SEPARATOR) .Append(" "); return sb.Append(enumerator.Next(), " ").Append(SEPARATOR_END, " ") .Append(enumerator.Next(), CLOSE) .ToString(); } } } class Program { static void Main(string[] args) { Action<string> print = Console.WriteLine; print( Solution.GetSequence(new string[] { "A", "B", "C", "D" }) ); print( Solution.GetSequence(new string[] { "A", "B" }) ); print( Solution.GetSequence(new string[] { "A" }) ); print( Solution.GetSequence(new string[] {}) ); print( Solution.GetSequence(null) ); } } }Anonymous
April 17, 2009
Just thought of another consideration: It's entirely possible that the input IEnumerable<string> can only be enumerated once, i.e. if it's from a Linq to SQL query. In this case, calling the Count(), First(), or Last() method before building the string will actually cause an exception to be thrown on the full pass. This is why I loathe unit testing - you can take the simplest code in the world, write twice as much testing code as implementation code, and still not cover the most elementary of failure conditions.Anonymous
April 17, 2009
@Aaron G - my team are heavy users -- and lovers -- of TDD. In the case you mention we would add another test that calls out this problem and then fix the code such that the new test passes while retaining the coverage offfered by the original tests. Noone expects that we can capture every possible problem that can arise, but as they are encountered they are handled in this manner.Anonymous
April 17, 2009
Mine is similiar to many of these: it's a single-pass with one flag. using System; using System.Text; using System.Collections.Generic; public class CommaSplicer { public static void Main(string[] args) { Random rand = new Random(); int numArrays = rand.Next(20); List<List<string>> l = new List<List<string>>(numArrays); for(int i = 0; i < numArrays; ++i) { List<string> li = new List<string>(i); for(int j = 0; j < i; ++j) li.Add(String.Intern(j.ToString())); l.Add(li); } List<string> resultList = new List<string>(numArrays); System.Diagnostics.Stopwatch sw = new System.Diagnostics.Stopwatch(); sw.Start(); foreach(List<string> ls in l) resultList.Add(CommaJoin(ls)); sw.Stop(); TimeSpan ts = sw.Elapsed; foreach(string s in resultList) Console.WriteLine(s); Console.WriteLine("Time to join {0} sets of strings: {1}", numArrays.ToString(), ts.ToString()); return; } public static string CommaJoin(IEnumerable<string> s) { StringBuilder temp = new StringBuilder(); IEnumerator<string> slist = s.GetEnumerator(); string a; bool firstComma = false; temp.Append('{'); if(slist.MoveNext()) // At least one item exists in the list for(a = slist.Current; ; a = slist.Current) { if(slist.MoveNext()) { // At least one more item exists in the list if(!firstComma) // First element of the list firstComma = true; else temp.Append(", "); temp.Append(a); } else { // No more items exist in the list if(firstComma) // Last element of the list temp.Append(" and "); temp.Append(a); break; } } temp.Append('}'); return temp.ToString(); } }Anonymous
April 17, 2009
Who needs state flags or re-parsing of the string? The good solutions are all taken, so I went for unique.. static string ParseFun(IEnumerable<string> input) { if (!input.Any()) { return "{}"; } StringBuilder sb = new StringBuilder(); sb.Append("}"); IEnumerable<string> s = input.Reverse().Select(x => x.Reverse()); Action<string> meth = str => { sb.Append(str); meth = str1 => { sb.Append(" dna "); sb.Append(str1); meth = str2 => { sb.Append(" ,"); sb.Append(str2); }; }; }; foreach (var item in s) { meth(item); } sb.Append("{"); return sb.ToString().Reverse(); } // boring string reverse extension method omitted.Anonymous
April 17, 2009
As I already noticed before, the flaw of my solution was in the use of sb.AppendFormat, definitely costly in terms of running time. Avoiding the unnecessary Format part, I am quite sure that all the other solutions require more time, and even much more memory. Testing it informally, 10 million calls with an array of 9 1-character strings, I get the result in less than 6 seconds, whereas the better solutions with lists/arrays can be around 8 seconds, with double memory requirements, maybe even more! Well, I know that space/time issues were not in the original requirements, but I also know that the original requirements were to use only the Enumerable methods, and most of the solutions are in fact using much more than that: mostly the extensions coming form Linq. That means to me not conceptually to-the-point: not portable to fw pre-3.5, to other languages/realms using the enumerable metaphors and so on! string Stringize(IEnumerable<string> list) { var sb = new StringBuilder("{"); var e = list.GetEnumerator(); if(e.MoveNext()) { sb.Append(e.Current); if(e.MoveNext()) { var current = e.Current; while(e.MoveNext()) { sb.Append(", ").Append(current); current = e.Current; } sb.Append(" and ").Append(current); } } return sb.Append("}").ToString(); }Anonymous
April 17, 2009
public static void Main(string[] args) { Console.WriteLine(Quibble(new List<String> {"one"})); Console.WriteLine(Quibble(new List<String> {"one", "two"})); Console.WriteLine(Quibble(new List<String> {"one", "two", "three"})); Console.WriteLine(Quibble(new List<String> {"one", "two", "three", "four"})); } private static String Quibble(IEnumerable<string> pStrings) { if (pStrings.Count() == 1) return String.Format("{0}{1}{2}", "{", pStrings.ElementAt(0), "}"); if (pStrings.Count() == 2) return String.Format("{0}{1}{2}{3}{4}", "{", pStrings.ElementAt(0), " and ", pStrings.ElementAt(1), "}"); return QuibbleOverTwo(pStrings); } private static String QuibbleOverTwo(IEnumerable<string> pStrings) { var aBuilder = new StringBuilder().Append("{"); var aStartIndexOfLastStr = -(pStrings.Last().Length + 1); foreach(var aString in pStrings) { aBuilder.Append(aString).Append(", "); aStartIndexOfLastStr += aString.Length + 2; } aBuilder.Remove(aStartIndexOfLastStr - 2, 1).Insert(aStartIndexOfLastStr - 1, "and "); return aBuilder.Remove(aBuilder.Length - 2, 2).Append("}").ToString(); }Anonymous
April 17, 2009
ErikF, I like your solution: no backtracking, single pass on input and output, both used as streams. You even stick to the methods of Enumerable but... why all these ifs in the inner loop? You have an if to check for a condition that can happen only on the first item, another one for a condition that can happen only on the last. If the strings enumerated are many, this will cost you a lot. And I do not like that break to exit from the loop. I'm not Dijkstra, but I believe that algs using few masked-gotos are much more readable and testable and... you know!Anonymous
April 17, 2009
@Filini Thanks for the feedback. For some reason, I figured Length was automatically kept up to date as items were added (not calculated) and would be simpler (one less variable, keeping the code terse) without adding a performance hit.Anonymous
April 17, 2009
In the spirit of violating YAGNI and over engineering that these sort of problems provide here is one that only requires going through the sequence once. It annotates the items with their index and whether it is the first, last or penultimate item and yields them as another IEnumerable. using System; using System.Text; using System.Collections.Generic; public class CommaQuibble { public static void Main() { List<string> list = new List<string>() { }; Console.WriteLine(ToList(list)); for(int i = 0; i <= 4; i += 1) { list.Add(i.ToString()); Console.WriteLine(ToList(list)); } } public static string ToList(IEnumerable<string> strings) { StringBuilder stringBuilder = new StringBuilder("{"); foreach(SequenceInfo<string> s in SequenceInfo<string>.GetSequenceInfo(strings)) { stringBuilder.Append(s.Value); if (s.IsPenultimate) { stringBuilder.Append(" and "); } else if (!s.IsLast) { stringBuilder.Append(", "); } } stringBuilder.Append("}"); return stringBuilder.ToString(); } } public class SequenceInfo<T> { private T _value; public T Value { get { return _value; } } private int index; public int Index { get { return index; } } public bool IsFirst { get { return (index == 0); } } private bool isLast; public bool IsLast { get { return isLast; } } private bool isPenultimate; public bool IsPenultimate { get { return isPenultimate; } } public SequenceInfo(T value, int index, bool isPenultimate, bool isLast) { this._value = value; this.index = index; this.isLast = isLast; this.isPenultimate = isPenultimate; } public static IEnumerable<SequenceInfo<T>> GetSequenceInfo(IEnumerable<T> sequence) { T current, next, nextnext; int index = 0; IEnumerator<T> enumerator = sequence.GetEnumerator(); if (enumerator.MoveNext()) { current = enumerator.Current; } else { yield break; } if (enumerator.MoveNext()) { next = enumerator.Current; } else { yield return new SequenceInfo<T>(current, index, false, true); yield break; } if (enumerator.MoveNext()) { nextnext = enumerator.Current; } else { yield return new SequenceInfo<T>(current, index, true, false); yield return new SequenceInfo<T>(next, index + 1, false, true); yield break; } while (true) { yield return new SequenceInfo<T>(current, index, false, false); current = next; next = nextnext; index += 1; if (enumerator.MoveNext()) { nextnext = enumerator.Current; } else { yield return new SequenceInfo<T>(current, index, true, false); yield return new SequenceInfo<T>(next, index + 1, false, true); yield break; } } } } @Aaron G: I don't think the goal should be to minimise the number of assignments or checks inside the loop. If it could be run on a large number of items I would argue the goal is to reduce the algorithmic complexity whilst still maintaining clarity (in this case only enumerating the sequence once or twice). Only if it still proves to be a performance bottleneck would you start to reduce assignments and checks. Obviously that's not a liscence to go mad :)Anonymous
April 17, 2009
And of couse the more serious reason than perfomance problems for not to use Count() or iterate twice, without populating whole sequence to list is instability of source: public static IEnumerable<string> GetSomeStrings() { var rnd = new Random(DateTime.Now.Ticks.GetHashCode()); var count = rnd.Next(10); for (var i = 0; i < count; i++) { yield return rnd.Next().ToString(); } }Anonymous
April 17, 2009
- There were very few F# programs
- I saw 1 F# program almost identical to this, but it had a bug (didn't handle 3 elements)
- Imperative programs are far too long and unreadable for this problem let answerToQuestion (items: IEnumerable<string>) = let rec joiner (items: IEnumerable<string>) = match Seq.to_list (items.Cast()) with | f :: s :: [] -> f + " and " + s | f :: [] -> f | f :: tl -> f + ", " + joiner tl | [] | _ -> "" "{" + (joiner items) + "}"
Anonymous
April 17, 2009
I reviewed my solution with one of my friends who works at Microsoft. He said my solution was fine, but the only thing he asked is, what is the cost of the algorithm. I hate to admit it, but at that point I was really focusing on code clarity and performance was not on my mind. Using the .Count() method makes it so that I have to iterate over the collection of items more than once. For small sets of data this is probably trivial, but as the data sets increase the performance of my algorithm decreases. This is why the scan forward methods are preferrable. So, although IMO the code I wrote may be a little easier to maintain - it is twice as slow. :| Lesson learned.Anonymous
April 17, 2009
@ICR: I considered using exactly the same approach - I have a similar "SmartEnumerable" class in my MiscUtil library (http://pobox.com/~skeet/csharp/miscutil) Note that in production code you should use a "using" statement for the IEnumerator<T> - that can be very important for sequences which use resources. My implementation is somewhat shorter though - please let me know if I've missed anything subtle! (I added the index part after first/last, which is why I've got a separate Boolean for it. I like your use of just the index.) public IEnumerator<Entry> GetEnumerator() { using (IEnumerator<T> enumerator = enumerable.GetEnumerator()) { if (!enumerator.MoveNext()) { yield break; } bool isFirst = true; bool isLast = false; int index=0; while (!isLast) { T current = enumerator.Current; isLast = !enumerator.MoveNext(); yield return new Entry(isFirst, isLast, current, index++); isFirst = false; } } }Anonymous
April 17, 2009
@ICR: Doh! I've just noticed one reason yours is more complicated - it has "IsPenultimate" as well as "IsLast". I suspect I could add that without quite as much code, but it's definitely additional complexity to consider. JonAnonymous
April 17, 2009
Take a look at Perl's Lingua::Conjunction module. It basically does what Eric's asked, plus it support a "phrase separator" in cases where the "word separator" (a comma in our case) already appears in any list element. So ['Jack, a boy', 'Jill, a girl', 'Spot, a dog'] becomes "Jack, a boy; Jill, a girl; and Spot, a dog". There's also a "penultimate" subroutine that performs the same function, except it excludes the final punctuation before the conjunction word. And of course this module allows you to pass in the word separator, phrase separator and the conjunction. All waaaaay out of scope for Eric's example, but it's something to consider if you're going to write/use this type of code in a production environment. http://search.cpan.org/dist/Lingua-Conjunction/Conjunction.pm I recently wrote a JScript and a Python version of Lingua::Conjunction but I don't have the source code with me at the moment. :-(Anonymous
April 17, 2009
NO C++ solution, yet... I need to fix this ;-) #include <algorithm> #include <string> #include <sstream> #include <iostream> #include <vector> #include <iterator> using namespace std; namespace Solution { template< class Elem, class TString = basic_string<Elem>, class TSequenceContainter = vector<TString> > class GetSequence { TString Open_, Close_, Separator_, Separator_End_; int min( int lVal, int rVal ){ return lVal < rVal ? lVal : rVal; } public: GetSequence(TString open, TString close, TString separator, TString separator_end) : Open_(open), Close_(close), Separator_(separator), Separator_End_(separator_end){} TString get(const TSequenceContainter& items) { TSequenceContainter::size_type itemsCount = items.size(); typedef ostream_iterator<TString, Elem> iter; basic_stringstream<Elem> ss; ss << Open_; copy( items.begin(), items.end() - min(2, itemsCount), iter( ss, Separator_.c_str() ) ); copy( items.end() - min(2, itemsCount), items.end() - min(1, itemsCount), iter( ss, Separator_End_.c_str() ) ); copy( items.end() - min(1, itemsCount), items.end(), iter( ss ) ); ss << Close_; return ss.str(); } }; GetSequence<char> seq_ASCII(){ return GetSequence<char>("{", "}", ", ", " and "); } GetSequence<wchar_t> seq_Unicode(){ return GetSequence<wchar_t>(L"{", L"}", L", ", L" and "); } } int main() { vector<string> x; x.push_back("A"); x.push_back("B"); x.push_back("C"); x.push_back("D"); cout << Solution::seq_ASCII().get( x ) << "n"; vector<string> y; y.push_back("A"); y.push_back("B"); cout << Solution::seq_ASCII().get( y ) << "n"; vector<string> z; z.push_back("A"); cout << Solution::seq_ASCII().get( z ) << "n"; vector<string> w; cout << Solution::seq_ASCII().get( w ) << "n"; vector<wstring> ux; ux.push_back(L"A"); ux.push_back(L"B"); ux.push_back(L"C"); ux.push_back(L"D"); wcout << Solution::seq_Unicode().get( ux ) << L"n"; vector<wstring> uy; uy.push_back(L"A"); uy.push_back(L"B"); wcout << Solution::seq_Unicode().get( uy ) << L"n"; vector<wstring> uz; uz.push_back(L"A"); wcout << Solution::seq_Unicode().get( uz ) << L"n"; vector<wstring> uw; wcout << Solution::seq_Unicode().get( uw ) << L"n"; }Anonymous
April 17, 2009
@Jagannath The input in the file would look as follows: abc bca , cab } bca Output: {abc,bca and cab} At least that is my "interpretation" of the problem: @Eric: Is this right?Anonymous
April 17, 2009
If you call XSLT a language, here's the solution in that: (test.xml) <?xml version="1.0"?> <?xml-stylesheet type="text/xsl" href="test.xsl"?> <root> <names> <name>Joe</name> <name>John</name> </names> <names> <name>Alice</name> </names> <names/> <names> <name>Bill</name> <name>Betty</name> <name>Bob</name> </names> </root> (test.xsl) <?xml version="1.0" encoding="UTF-8"?> <xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="1.1"> <xsl:output method="text"/> <xsl:template match="/"> <xsl:apply-templates/> </xsl:template> <xsl:template match="names"> Original: {<xsl:apply-templates select="name"/>} Delimited: <xsl:call-template name="do-commas"> <xsl:with-param name="values" select="name"/> </xsl:call-template> </xsl:template> <xsl:template name="do-commas"> <xsl:param name="values"/> <xsl:text>{</xsl:text> <xsl:for-each select="$values"> <xsl:choose> <xsl:when test="count($values)>1 and position()=count($values)"> <xsl:text> and </xsl:text> </xsl:when> <xsl:when test="position()>1"> <xsl:text>, </xsl:text> </xsl:when> </xsl:choose> <xsl:value-of select="."/> </xsl:for-each> <xsl:text>}</xsl:text> </xsl:template> </xsl:stylesheet>Anonymous
April 17, 2009
This is just my brain dump -- a quick attempt -- with scant regard to refining it (which I'm sure is possible in truckloads :-)). I just felt that a few extra operations would not be too expensive when compensating for the "if" inside the loop. public string CommaDelimited(IEnumerable<string> someCollection) { StringBuilder sb = new StringBuilder(); int count = 0; foreach (string element in someCollection) { sb.Append(element).Append(","); //register operation should not be expensive count ++; } //remove trailing comma if (sb.Length > 0) { sb.Remove(sb.Length - 1, 1); } if (count > 1) { int index = sb.Length - 1; while (sb[index] != ',') { index--; } sb.Remove(index, 1); sb.Insert(index, " And "); } return sb.Insert(0, "{").Append("}").ToString(); }Anonymous
April 17, 2009
I am sorry. This is my third post. I am confused by the amount of code everyone is writing. As far as I understood, this is my program which Joins the string with and without duplicate entries. I considered comma and brace as strings and did not ignore them. @Eric. Sorry for the duplicate entries if I did not understand the problem. using System; using System.Diagnostics; using System.Collections.Generic; using System.Collections; using System.Linq; using System.Text; namespace CommaQuibble { static class Program { static void Main(string[] args) { IEnumerable<string> elems = BuildStringArray(); System.Diagnostics.Stopwatch sw = new Stopwatch(); sw.Start(); string s = JoinWithDups(elems.ToArray<string>(), ",", " and "); string s2 = JoinWithoutDups(elems.ToArray<string>(), ",", " and "); TimeSpan ts = sw.Elapsed; Console.WriteLine(s); Console.WriteLine(ts.TotalSeconds); } static IEnumerable<string> BuildStringArray() { string alphabets = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; Random random = new Random(DateTime.Now.Ticks.GetHashCode()); for (int i = 0; i < 100000; i++) { int min = random.Next(10, 25); int max = random.Next(min, 25); yield return alphabets.Substring(min, max - min + 1); } } public static string JoinWithoutDups (string[] source, string delimeter1, string delimeter2) { int NumElem = source.Count(); StringBuilder finalString = new StringBuilder("{"); Dictionary<string, int> duplicates = new Dictionary<string, int>(); for (int i = 0; i < NumElem - 1;) { if (duplicates.ContainsKey(source[i]) == false) { finalString.Append(string.Format("{0}{1}", source[i], delimeter1)); duplicates[source[i]] = 1; } i++; } if (NumElem > 0 && (duplicates.ContainsKey(source[NumElem - 1]) == false)) { finalString.Append(delimeter2); finalString.Append(source[NumElem - 1]); } finalString.Append("}"); return finalString.ToString(); } public static string JoinWithDups (string[] source, string delimeter1, string delimeter2) { int NumElem = source.Count(); StringBuilder finalString = new StringBuilder("{"); for (int i = 0; i < NumElem - 1; ) { finalString.Append(string.Format("{0}{1}", source[i], delimeter1)); i++; } if (NumElem > 0) { finalString.Append(delimeter2); finalString.Append(source[NumElem - 1]); } finalString.Append("}"); return finalString.ToString(); } } }Anonymous
April 17, 2009
@Eric: My apologies for a duplicate submission. Looks like my "interpretation" was a "misinterpretation", in that I overlooked the fact that the words themselves can contain "{" "," "}" My interpretation was that the word could be either a complete word without these characters or the word is a special character or a combination of these special characters thereof. I've updated my solution to correct my "misinterpretation" and I'm re-submitting the solution. (a couple of small changes) Hope multiple submission are not a ground for dismissals :-) The file I'm using "Words.txt" can be downloaded from <a href="http://geocities.com/linqrocks/Words.txt">here</a> The performance for approx. 160 words is 53 seconds. I also tried it for 173,000 words(which can be downloaded from <a href="http://geocities.com/linqrocks/Word.txt">here</a> and the performance was absymal mainly due to the 'Aggregate' function I'm using. Any tips on making this more performant would be MUCH appreciated. Here goes: public static IEnumerable<string> GetWords(string fileName) { using (StreamReader reader = new StreamReader(fileName)) { while(!reader.EndOfStream) { string word = reader.ReadLine(); yield return word; } yield break; } } public static string Concat(IEnumerable<string> words) { string[] excludesStrArr = { ",", "{", "}", "" }; char[] excludesChrArr = excludesStrArr.Except(Enumerable.Repeat(excludesStrArr.Last(),1)) .Select(s => Convert.ToChar(s)).ToArray(); string str; words = words.Select(s => s.Trim().Trim(excludesChrArr)).Except(excludesStrArr); if(words.SequenceEqual(Enumerable.Empty<string>())) { str = "{}"; return str; } if(words.ElementAtOrDefault(0) != null && words.ElementAtOrDefault(1) == null) { str = words.SingleOrDefault(); return str; } if (words.ElementAtOrDefault(1) != null && words.ElementAtOrDefault(2) == null) { str = string.Concat("{", words.ElementAt(0), " and ", words.ElementAt(1), "}"); return str; } else { StringBuilder sb = new StringBuilder(); string s = words.Aggregate((w1, w2) => string.Concat(w1, ",", w2)); sb.Append(s); sb.Replace(",", " and " , s.LastIndexOfAny(new char[]{','}),1); str = sb.ToString(); str = string.Concat("{", str, "}"); return str; } } static void Main(string[] args) { Stopwatch watch = new Stopwatch(); var words = GetWords("Words.txt"); watch.Start(); Console.WriteLine(Concat(words)); watch.Stop(); Debug.WriteLine(string.Format("StrConcat took {0} Milliseconds", watch.Elapsed.Milliseconds)); }Anonymous
April 17, 2009
@Jagannath You are correct, I misinterpreted the requirement, please check out the previous comment in which I've updated my solution to include the "fix" The file I'm using can be downloaded from http://geocities.com/linqrocks/Word.txt Thanks.Anonymous
April 17, 2009
I thought IronPython was supposed to plug these holes!Anonymous
April 17, 2009
@Eric: P.S. That's 53 Milliseconds not SecondsAnonymous
April 17, 2009
If your into problem solving eric (and we all are) here's a good question: http://acm.tju.edu.cn/toj/showp3036.htmlAnonymous
April 17, 2009
Ive used the following two steps:
- Insert commas at every alternate position in the list
- replaces the last comma with an "and". Code: public string insertCommas(IEnumerable<string> stringList) { IEnumerable<string> listWithCommas = _insertCommas(stringList); StringBuilder builder = new StringBuilder(); builder.Append("{"); foreach (string item in listWithCommas) { builder.Append(item); } builder.Append("}"); return builder.ToString(); } private IEnumerable<string> _insertCommas(IEnumerable<string> stringList) { List<string> list = stringList.ToList(); if (list.Count - 1 <= 0) return stringList; for (int index = 1; index < list.Count; index+=2) { list.Insert(index, ", "); } list[list.LastIndexOf(", ")] = " and "; return list; }
Anonymous
April 17, 2009
I guess I'm a glutton for trying to improve performance, so I decided to take another hack at it :) I was able to bring the time down to 0.36 seconds for approx 173,528 words: Here are the test files I used: http://geocities.com/linqrocks/Word.txt http://geocities.com/linqrocks/Words.txt Code follows: public static IEnumerable<string> GetWords(string fileName) { using (StreamReader reader = new StreamReader(fileName)) { while(!reader.EndOfStream) { string word = reader.ReadLine(); yield return word; } yield break; } } public static string Concat(IEnumerable<string> words) { string[] excludesStrArr = { ",", "{", "}", "" }; char[] excludesChrArr = excludesStrArr.Except(Enumerable.Repeat(excludesStrArr.Last(),1)) .Select(s => Convert.ToChar(s)).ToArray(); string str; words = words.Select(s => s.Trim().Trim(excludesChrArr)).Except(excludesStrArr); if(words.SequenceEqual(Enumerable.Empty<string>())) { str = "{}"; return str; } if(words.ElementAtOrDefault(0) != null && words.ElementAtOrDefault(1) == null) { str = words.SingleOrDefault(); return str; } if (words.ElementAtOrDefault(1) != null && words.ElementAtOrDefault(2) == null) { str = string.Concat("{", words.ElementAt(0), " and ", words.ElementAt(1), "}"); return str; } else { StringBuilder sb = new StringBuilder("{"); string s = words.Aggregate((w1, w2) => {sb.Append(w1); sb.Append(","); sb.Append(w2); return null; }); int index = sb.Length - 1; while (sb[index] != ',') { index--; } sb.Replace(",", " and ", index, 1); sb.Append("}"); str = sb.ToString(); return str; } } static void Main(string[] args) { Stopwatch watch = new Stopwatch(); var words = GetWords("Word.txt"); watch.Start(); string str = Concat(words); watch.Stop(); Debug.WriteLine(string.Format("StrConcat took {0} Milliseconds", watch.Elapsed.Milliseconds)); Console.WriteLine(str); }Anonymous
April 17, 2009
A nice old school imperative style solution: public static string ConcatenateStringsWithCommas(IEnumerable<string> enumerable) { if (enumerable == null) { return null; } int idx = 0, len = 0; string comma = ", "; StringBuilder sb = new StringBuilder("{"); foreach (string s in enumerable) { if(idx++ >= 1) sb.Append(comma); sb.Append(s); len = s.Length + comma.Length; } if(idx > 1) sb.Replace(comma, " AND ", sb.Length - len, comma.Length); sb.Append("}"); return sb.ToString(); }Anonymous
April 17, 2009
How about just this simple solution... string[] elements = new string[] { "ABC", "DEF", "GC", "Z" };
string s = string.Join(", ", elements);
Console.WriteLine(Regex.Replace(s, "^(., [^,])(?:, )([^,]*)$", "$1 and $2")); What if the strings contain commas? And where are the braces? -- EricAnonymous
April 18, 2009
string[] elements1 = new string[] { "ABC" };
string[] elements2 = new string[] { "ABC", "DEF" };
string[] elements3 = new string[] { "ABC", "DEF", "GC", "Z" };
string s = string.Join(", ", elements1);
Console.WriteLine(Regex.Replace(s, "^(., )?([^,])(?:, )([^,])$", "$1$2 and $3"));
s = string.Join(", ", elements2);
Console.WriteLine(Regex.Replace(s, "^(., )?([^,])(?:, )([^,])$", "$1$2 and $3"));
s = string.Join(", ", elements3);
Console.WriteLine(Regex.Replace(s, "^(., )?([^,])(?:, )([^,]*)$", "$1$2 and $3")); Just realized that the code did not work in all scenarios... the above would though. Again, what if one of the strings contained commas? Suppose the strings were "(1, 2)", "(2, 4)" and "(4, 8)", for example? And still no braces! -- EricAnonymous
April 18, 2009
The comment has been removedAnonymous
April 18, 2009
In general converting the IEnumerable<string> to a list/array will definitely have significant perf. impact. The more you can stay in the IEnumerable land without explicitly materializing the underlying objects, the better off you are. Imagine if your data source had 1 million words, you would be shot from the get go. Also bear in mind that when you perform any operations on the "String" class you create a new "String" since strings are immutable so if you're doing any search/replace/concat operations on a large string good luck! Just my 2 centsAnonymous
April 18, 2009
It's been several days since I posted my first try (and my attempt at a funny try), and having some free time today (having just finished with finals! :), I thought maybe I'd try doing it only using the IEnumerable (not converting to a list or array) without being too fancy. Here's what I came up with; it seems to work pretty well and didn't really take that long to write (the whole program follows, explanation below): using System; using System.Collections.Generic; using System.Linq; namespace Scratch { class Program { static List<string> words; static void Main(string[] args) { //string[] input = { }; //string[] input = { "ABC" }; //string[] input = { "ABC", "DEF" }; //string[] input = { "ABC", "DEF", "G", "H", "I" }; string output = null; var watch = new System.Diagnostics.Stopwatch(); IEnumerable<string> input = GetWords("english.txt"); watch.Start(); output = FormatEnumerableStrings(input); watch.Stop(); Console.WriteLine(String.Format("ToList() time elapsed: {0} milliseconds", watch.ElapsedMilliseconds)); watch.Reset(); watch.Start(); output = FormatEnumerableStrings2(input); watch.Stop(); Console.WriteLine(String.Format("Enumerative time elapsed: {0} milliseconds", watch.ElapsedMilliseconds)); //Console.WriteLine(output); } // New function that does not use ToList() static string FormatEnumerableStrings2(IEnumerable<string> input) { var sb = new System.Text.StringBuilder("{"); var it = input.GetEnumerator(); sb.Append(input.FirstOrDefault()); it.MoveNext(); if (it.MoveNext()) { string next = it.Current; string prev = next; while (it.MoveNext()) { prev = next; next = it.Current; sb.Append(", "); sb.Append(prev); } sb.Append(" and "); sb.Append(next); } return sb.Append("}").ToString(); } // Initial function that uses ToList() static string FormatEnumerableStrings(IEnumerable<string> input) { var sb = new System.Text.StringBuilder("{"); var strings = input.ToList(); int count = strings.Count; if (count > 0) { sb.Append(strings[0]); for (int i = 1; i < count - 1; i++) { sb.Append(", "); sb.Append(strings[i]); } if (count > 1) { sb.Append(" and "); sb.Append(strings[count - 1]); } } return sb.Append("}").ToString(); } static IEnumerable<string> GetWords(string file) { words = new List<string>(); string line; using (var txt = new System.IO.StreamReader(file)) { while ((line = txt.ReadLine()) != null) words.Add(line); } return words.AsEnumerable(); } } } I'm pretty sure this is less clear than my initial iterative approach (my April 15, 2009 6:11 PM post) as this has two variables keeping track of words instead of just one. When I did some testing on a huge word list[1]; however, I found that the performance of my initial approach using ToList() is almost the exact same as the second attemp without ToList(). Looking at the MSDN page[2] for it I don't see any mention of time complexity (some pages explicitly mention a Big Oh), however I suppose this may be due to how I'm using a List as the source for my IEnumerable (?). On my machine I can run both algorithms against 192,718 words[2] in about 0.035 seconds. Hopefully I haven't made any completely obvious mistakes. [1]: english-words.95 in http://downloads.sourceforge.net/wordlist/scowl-6.zip [2]: http://msdn.microsoft.com/en-us/library/bb342261.aspxAnonymous
April 18, 2009
Dur. Probably obvious, but "192,718 words" should reference the first footnote, not the second.Anonymous
April 18, 2009
The comment has been removedAnonymous
April 19, 2009
@The First Nick When you are testing the ToList solution are you testing it by passing in a List<T> cast to an IEnumerable<T>? I'm fairly sure ToList has a shortcut that checks if the object is really a list. Try it with passing in an IEnumerable that isn't actually a List (a Queue would probably work quite well for a quick check).Anonymous
April 19, 2009
The comment has been removedAnonymous
April 19, 2009
// Should be efficient due to immutable nature of string in .NET public static class CommaQuibbler { public static string Quibble(IEnumerable<string> words) { string result = String.Empty, previousResult = String.Empty, lastWord = String.Empty, glue = ", ", lastGlue = " and "; int count = 0; foreach (string word in words) { previousResult = result; result += (count++ > 0 ? glue : String.Empty) + word; lastWord = word; } if (previousResult != String.Empty) result = previousResult + lastGlue + lastWord; return '{' + result + '}'; } } /* PS. Special thanks to you for your posts about immutability!I was amazed when immutable List saved me about 2Gb of RAM for one computational task! */
Anonymous
April 19, 2009
ICR: Interesting thought. I did some additional testing with an enormous word list[1] containing about 2.7 million words/phrases with the following results (an average of 10 runs) using my posted code: Source: List<string> ToList() time: 295 ms Enumerable time: 320 ms Source: Queue<string> ToList() time: 415 ms Enumerable time: 340 ms Source: HashSet<string> ToList() time: 325 ms Enumerable time: 330 ms So yes, it looks like calling ToList() on an IEnumerable<T> backed by a List<T> is faster than other collections, but not by much. One thing I did notice is that I can cut both times by about 25-35% by defining the StringBuilder's initial capacity, though I don't think that's really relevant to this discussion. In all cases, List<T> is the best performer, followed by HashSet<T>, and finally Queue<T>. Meh, I suppose this is getting beyond the point of Eric's post, but I think it's interesting anyway. Better than sitting around bored on a Sunday afternoon. [1]: http://www.zinkwazi.com/tools/wordlist.zip (30 MB uncompressed)Anonymous
April 20, 2009
@First Nick: your FormatEnumerableStrings2 is just the same as mine but I don't agree on some passages:
- You use the .FirstOrDefault() method, but this is not part of the IEnumerable definition: it comes from the Linq extensions... you have avoided all the others extensions, why do you think you still need this? It should be better to append the Current under an "if(MoveNext)"! Remember: the iterator is initially positioned before the first (if ever) element.
- In the loop you have two string vars, but you really need only one: see what happens when you assign Current after appending the previous instance!
- I don't like the two MoveNext at the beginning: maybe this is not the case for the IEnumerator, but there are programming metaphors in which you have to catch the first "exception", because a subsequent call will return another "state". Suppose the list was empty: the first MoveNext returns false, but you lose the return status, and you have to call it again to get an information that was already given to you! The points 1 and 3 are actually for the same problem: if you simply put the first append after an "if(MoveNext)" (i.e.: my solution), I think you just have a more robust code!
Anonymous
April 20, 2009
The comment has been removedAnonymous
April 20, 2009
First Try, I'm sure GetJoinedValue can be prettied up a bit... [TestFixture] public class IEnumberableExtensionsTestFixture { private IEnumerable<String> testEnumerable; private string expectedResults; [TearDown] public void AssertAll() { var actualResults = testEnumerable.ToNonOxfordCommaString(); Assert.That(actualResults, Is.EqualTo(expectedResults)); } [Test] public void an_empty_string_returns_empty_curly_braces() { testEnumerable = new List<String> { "" }; expectedResults = "{}"; } [Test] public void a_single_value_is_wrapped_in_curly_braces() { testEnumerable = new List<String> {"ABC"}; expectedResults = "{ABC}"; } [Test] public void two_values_uses_and_to_join() { testEnumerable = new List<String> {"ABC", "DEF"}; expectedResults = "{ABC and DEF}"; } [Test] public void more_than_2_values_use_commas_with_and_as_the_last_value() { testEnumerable = new List<String> { "ABC", "DEF", "G", "H" }; expectedResults = "{ABC,DEF,G and H}"; } } public static class IEnumberableExtensions { /// <summary> /// Retuns a joined string with "and" as the last join. /// </summary> /// <returns></returns> public static string ToNonOxfordCommaString(this IEnumerable<String> enumberable) { var joinedValue = GetJoinedValue(enumberable); return String.Format("{{{0}}}",joinedValue); } private static string GetJoinedValue(IEnumerable<string> enumberable) { var totalCount = enumberable.Count(); var lastValue = enumberable.ToList()[totalCount - 1]; switch (totalCount) { case 0: return String.Empty; case 1: return lastValue; ); default: return String.Join(",", enumberable.Take(totalCount - 1).ToArray() ) + " and " + lastValue; } } }Anonymous
April 20, 2009
Hi. This is my version of solution. Hope you like it :) Tried to make it as simple as possible. using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.Text; namespace ConsoleApplication2 { class Program { static void Main(string[] args) { Debug.Assert(new string[]{}.AddCommas()=="{}"); Debug.Assert(new string[]{"ABC"}.AddCommas()=="{ABC}"); Debug.Assert(new string[]{"ABC","DEF"}.AddCommas()=="{ABC and DEF}"); Debug.Assert(new string[]{"ABC","DEF","G","H"}.AddCommas()=="{ABC, DEF, G and H}"); } } } /* (1) If the sequence is empty then the resulting string is "{}". (2) If the sequence is a single item "ABC" then the resulting string is "{ABC}". (3) If the sequence is the two item sequence "ABC", "DEF" then the resulting string is "{ABC and DEF}". (4) If the sequence has more than two items, say, "ABC", "DEF", "G", "H" then the resulting string is "{ABC, DEF, G and H}". (Note: no Oxford comma!) */ static class StringCommer { public static string AddCommas(this IEnumerable<string> strings) { int lastElementIndex = strings.Count(); if (lastElementIndex == 0) return "{}"; StringBuilder resultBuilder = new StringBuilder(); bool isFirst = true; int currentElementIndex = 0; resultBuilder.Append("{"); foreach (var currentItem in strings) { currentElementIndex++; if (isFirst) isFirst = false; else { if (currentElementIndex != lastElementIndex) resultBuilder.Append(", "); else resultBuilder.Append(" and "); } resultBuilder.Append(currentItem); } resultBuilder.Append("}"); return resultBuilder.ToString(); } }Anonymous
April 20, 2009
p.s. Yes, I can remove isFirst variable, with something like if (currentElementIndex++ != 0) { if (currentElementIndex != lastElementIndex) resultBuilder.Append(", "); else resultBuilder.Append(" and "); } but don't wan't to do it :)Anonymous
April 20, 2009
Nothing fancy. May not be too efficient, but fairly easy to read. public static string CombineStringWithEnglishSyntax(IEnumerable<String> strings) { StringBuilder builder = new StringBuilder(); // Add the starting { builder.Append("{"); foreach (string item in strings) { builder.Append(item); builder.Append(", "); } // Add the ending } builder.Append("}"); // Get rid of the extra comma added at the end. // If there were no items added, this does nothing builder.Replace(", }", "}"); // Find the last instance of ", " if there is one. int lastCommaIndex = builder.ToString().LastIndexOf(", "); if (lastCommaIndex >= 0) { // Replace the last instance of ", " with " and ". builder.Replace(", ", " and ", lastCommaIndex, 2); } return builder.ToString(); }Anonymous
April 20, 2009
pingback from http://blog.elliottohara.com/2009/04/eric-lipperts-comma-quibbling-challenge.htmlAnonymous
April 20, 2009
Here's a version as a batch file! (Put the elements that you want to join as arguments to the batch file.) I can't think of any plausible reason why you would do this except to show that you can. It seems wrong to do this in a batch file though.... @echo off rem Put the beginning curly brace and starting value, if any set sp={%1 shift rem Zero or one elements: don't do any further processing if !%1==! goto end :mainloop rem Two or more elements: use "and" for the last element if !%2==! goto final rem Attach the elements with a comma set sp=%sp%, %1 shift goto mainloop :final rem Attach the final elements with an "and" and fallthrough set sp=%sp% and %1 :end echo %sp%}Anonymous
April 20, 2009
Sorry I didn't read them all before posting (but I did read quite a few) so I hope this isn't a reapeat. using System; using System.Collections.Generic; using System.Linq; using System.Text; namespace CommaAndTestApp { class CommaAndTest { static void Main(string[] args) { Console.WriteLine(GetPrettyString(GetStrings(0))); Console.WriteLine(GetPrettyString(GetStrings(1))); Console.WriteLine(GetPrettyString(GetStrings(2))); Console.WriteLine(GetPrettyString(GetStrings(3))); } static string GetPrettyString(IEnumerable<string> items) { if (items == null) throw new Exception("items is null"); StringBuilder sb = new StringBuilder(); int i = items.Count(); sb.Append("{"); foreach (string s in items) { sb.Append(s + (--i > 0 ? (i == 1 ? " and " : ", ") : string.Empty)); } sb.Append("}"); return sb.ToString(); } static IEnumerable<string> GetStrings(int HowMany) { string[] retVal = { "a", "b", "c"}; return retVal.Take(HowMany); } } } Output: {} {a} {a and b} {a, b and c}Anonymous
April 20, 2009
A variation using 2 stacks. using System; using System.Collections.Generic; using System.Text; namespace ConsoleApplication1 { static class Program { static void Main(string[] args) { Console.WriteLine("{} = " + (new string[0]).Quibble()); Console.WriteLine("{ABC} = " + (new string[] { "ABC" }).Quibble()); Console.WriteLine("{ABC and DEF} = " + (new string[] { "ABC", "DEF" }).Quibble()); Console.WriteLine("{ABC, DEF, G and H} = " + (new string[] { "ABC", "DEF", "G", "H" }).Quibble()); Console.ReadKey(); } public static string Quibble(this IEnumerable<string> items) { var backwards = new LinkedList<string>(); var forwards = new LinkedList<string>(); foreach (var s in items) backwards.AddFirst(s); // "}" is always the last character in the result string. push it forwards.AddFirst("}"); // Initially no counter was used. However the choice between " and " and ", ", // and when to pop the final seperator was based on unintuative comparisons // to stack.Count so a counter was introduced. var count = 0; foreach (string s in backwards) { forwards.AddFirst(s); // push " and " the first time, ", " every other time. forwards.AddFirst(count == 0 ? " and " : ", "); count++; } // if we've pushed at least one pair of items, the top of the stack // is a garbage seperator. Pop it. if (count > 0) forwards.RemoveFirst(); // The first item is always a "{". Push it. forwards.AddFirst("{"); StringBuilder sb = new StringBuilder(); foreach (string s in forwards) sb.Append(s); return sb.ToString(); } } }Anonymous
April 20, 2009
@Elliott O'Hara: Be careful... "new List<String>{""}" is NOT an empty list, but a list with ONE element, the empty string. Even you unit test is wrong! Your code fails (indexing the list with -1) when the REAL empty list is passed! @Ed (and others using LastIndexOf): Remember... the strings in the list could contain commas, and maybe even the substring ", "! What do you think is happening to your code if the last string in the list passed is, say, "test, other test"?Anonymous
April 21, 2009
//I was trying to go for something completely different here, not efficiency. using System; using System.Collections.Generic; using System.Text; using System.Text.RegularExpressions; namespace CommaQuibler { class Program { static void Main(string[] args) { Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { })); Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { "ABC" })); Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { "ABC", "DEF" })); Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { "ABC", "DEF", "G", "H" })); Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { "}", "{", "", ",", "{abc}", "},{" })); Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { "3}" })); Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { "{3" })); Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { "{0}", "{1}", "{2}", "{3}", "{4}" })); Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { "{0}{1}{2}{3}{4}" })); Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { "4}" })); Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { "{4{}3443{}}3}3}", })); Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { "}{}3}4{3}" })); Console.WriteLine(CommatizeWithNoOxfordComma(new string[] { "{4" })); Console.ReadKey(); } private static string CommatizeWithNoOxfordComma(IEnumerable<string> strings) { StringBuilder sb = new StringBuilder(); foreach (string s in strings) { string h; sb = new StringBuilder(string.Format(sb.ToString(), "{0}", "{0}", "{1}", "{3}", "{4}")); h = s.Replace("{","{3}"); h = Regex.Replace(h,"(?<!{3)}","{4}"); sb.Append(h); sb.Append("{2}"); } sb.Insert(0,"{3}"); sb.Append("{4}"); return string.Format(sb.ToString(), new string[] { ", ", " and ", "", "{", "}" }); } } }Anonymous
April 21, 2009
@ptoniolo, yeah, already caught that (of course, after I posted)... I fixed it on my blog, but here's the code. public void no_sequences_does_not_throw_argument_out_of_range() 53 { 54 testEnumerable = new List<String>(); 55 expectedResults = "{}"; 56 } 25 private static string GetJoinedValue(IEnumerable<string> enumberable) 26 { 27 var totalCount = enumberable.Count(); 28 var lastValue = totalCount == 0 ? 29 String.Empty: 30 enumberable.ToArray()[totalCount - 1]; 31 32 switch (totalCount) 33 { 34 case 0: 35 case 1: 36 return lastValue; 37 default: 38 return String.Join(",", 39 enumberable.Take(totalCount - 1).ToArray() 40 ) + " and " + lastValue; 41 42 } 43 } Challenge now is to use polymorphic behavior and get rid of the silly switch (that could now just be an if).Anonymous
April 21, 2009
Just saw your post (a bit late).. I see there's great response! Here is my implementation, though. class Program { static void Main(string[] args) { // There’s no size limit on the sequence; // it could be tiny, // it could be thousands of strings. // But it will be finite. Console.WriteLine(CommaQuibbling(GetStringSeq(0))); Console.WriteLine(CommaQuibbling(GetStringSeq(1))); Console.WriteLine(CommaQuibbling(GetStringSeq(5))); Console.WriteLine(CommaQuibbling(GetStringSeq(10))); } static string CommaQuibbling(IEnumerable<string> seq) { string result = string.Empty; int seqCount = seq.Count(); if (seqCount > 0) { var newSeq = seq.Select((item, index) => { if (index == 0) return item; else if (index < seqCount - 1) return ", " + item; else return " and " + item; }); result = string.Join("", newSeq.ToArray()); } return "{" + result + "}"; } static IEnumerable<string> GetStringSeq(int length) { while (length-- > 0) yield return "ABC"; } }Anonymous
April 21, 2009
And the winner is RickDaleyOptimized, I think. Very readable and avoiding redundant call to .Count() on Enumerable.Anonymous
April 21, 2009
About Rick Dailey's solution... well, I felt a little unconfortable with it, and now I understand why! The original GetCombined() fails if the first string of the list is the empty string: the list {"","x"} should produce "{ and x}", but the result is only "{x}"; the first null string does not change the length of the "builder", that has still the same length as the "opening". Thus, GetCombinedOptimized() is a better solution, not only because it does not call the Length so many times, it also fixes this subtle bug! Anyway, I still believe that it is not a good idea to put two "branch" statements (an if and a ternary operator) inside the inner loop.Anonymous
April 21, 2009
Ok, my try. Basically I repacked the enumerator. --- CODE --- public static String Solution(IEnumerable<String> words) { var sb = new StringBuilder(); foreach (var word in Prettyfier(words)) { sb.Append(word); } return sb.ToString(); } public static IEnumerable<String> Prettyfier(IEnumerable<String> words) { yield return "{"; var a = new String[3]; foreach (var word in words) { a[0] = a[1]; a[1] = a[2]; a[2] = word; if (a[0] != null) yield return a[0] + ", "; } if (a[1] != null) yield return a[1] + " and "; if (a[2] != null) yield return a[2]; yield return "}"; } --- END CODE --- Any comments? LP, DejanAnonymous
April 21, 2009
Note: all solutions that checks if current element is not the first one by StringBuilder length has the bug that if sequense begins with set of empty strings, then some commas will be lost. For example for {"", "", "", ""} correct ouput should be "{, , and }" but such solutions will produce something like "{}".Anonymous
April 21, 2009
Incorporating everyone's great suggestions: static string GetCombined(IEnumerable<string> strings) { var builder = new StringBuilder('{'); using(var enumerator = strings.GetEnumerator()) { bool hasNext = enumerator.MoveNext(); bool first = true; while (hasNext) { string s = enumerator.Current; hasNext = enumerator.MoveNext(); if (first) first = false; else builder.Append(hasNext ? ", " : " and "); builder.Append(s); } } builder.Append('}'); return builder.ToString(); }Anonymous
April 21, 2009
The comment has been removedAnonymous
April 22, 2009
My second try for what I believe is very readable and maintainable code public static string CommaQuibbling(this IEnumerable<string> items) { var builder = new StringBuilder(); using (IEnumerator<string> enumtor = items.GetEnumerator()) return builder.AppendBracketed(enumtor).ToString(); } private static StringBuilder AppendBracketed(this StringBuilder builder, IEnumerator<string> enumtor) { return builder.Append('{').AppendZeroOrMore(enumtor).Append('}'); } private static StringBuilder AppendZeroOrMore(this StringBuilder builder, IEnumerator<string> enumtor) { if (!enumtor.MoveNext()) return builder; else return builder.AppendOneOrMore(enumtor.Current, enumtor); } private static StringBuilder AppendOneOrMore(this StringBuilder builder, string first, IEnumerator<string> enumtor) { if (!enumtor.MoveNext()) return builder.Append(first); else return builder.AppendTwoOrMore(first, enumtor.Current, enumtor); } private static StringBuilder AppendTwoOrMore(this StringBuilder builder, string first, string second, IEnumerator<string> enumtor) { while (enumtor.MoveNext()) { builder = builder.Append(first).Append(", "); first = second; second = enumtor.Current; } return builder.Append(first).Append(" and ").Append(second); }Anonymous
April 22, 2009
Olivier's arguments in favor of terseness would be more persuasive if his code worked. It has a bug for the case of inputs sequences of length 2. @Dave "More lines equals more bugs": Except when too few lines equals more bugs. Cf bathtub curve. I'm not seeing the bug. I tried compiling and running the code on a few cases and it worked fine. What's the repro you have in mind? -- EricAnonymous
April 22, 2009
static string JoinWords(IEnumerable<string> words) { string wordList = String.Empty; int count = words.Count(); switch (count) { case 0: break; case 1: wordList = words.First(); break; default: wordList = string.Join(", ", words.Take(count - 1).ToArray()) + " and " + words.Last(); break; } return "{" + wordList + "}"; }Anonymous
April 22, 2009
static string JoinWords(IEnumerable<string> words) { var wordList = new StringBuilder(); string lastWord = null; string andSeparator = String.Empty; string commaSeparator = String.Empty; foreach (string str in words) { if (lastWord != null) { wordList.Append(commaSeparator); wordList.Append(lastWord); andSeparator = " and "; commaSeparator = ", "; } lastWord = str; } return "{" + wordList.ToString() + andSeparator + (lastWord ?? String.Empty) + "}"; }Anonymous
April 22, 2009
Uh oh. The bug was not in Olivier's code but in my misreading of the spec, and thus my test suite. Somehow I thought "and" was not supposed to materialize until the list contained 3+ items. No clue where I got that from. Olivier--I don't know if you'd consider terseness as much of a virtue in apologies as in code, but if so: MY BAD. Eric--thanks for treating my post with less mischief than it had initiated or, for that matter, deserved. This is a first class blog and can reasonably be assumed to have a first class audience; I should have n-tuple checked my math before bringing the snark.Anonymous
April 23, 2009
If I had a lot of lookaheads in my program, I would probably make an enumerator that let me do that. I added an unchecked Current and Lookahead for routines that don't need the documented IEnumerator behaviour. This is probably not the most efficient implementation, but it worked OK for this application. using System; using System.Collections; using System.Collections.Generic; using System.Text; /// LookaheadEnumerator: Provides an enumerator that allows a program to "peek" into the next element public sealed class LookaheadEnumerator<T>: IEnumerator<T>, System.Collections.IEnumerator { private IEnumerator<T> m_enum; private bool m_hasNext; private bool m_pastBounds; private bool m_atStart; private T m_current; private T m_lookahead; public LookaheadEnumerator(IEnumerator<T> e) { m_enum = e; Init(); } public void Reset() { m_enum.Reset(); Init(); } private void Init() { m_atStart = m_pastBounds = true; m_hasNext = m_enum.MoveNext(); m_current = default(T); m_lookahead = (m_hasNext) ? m_enum.Current : default(T); } public bool MoveNext() { m_current = m_lookahead; if(m_hasNext) { m_hasNext = m_enum.MoveNext(); m_atStart = m_pastBounds = false; m_lookahead = (m_hasNext) ? m_enum.Current : default(T); } else // No more elements exist m_pastBounds = true; return !m_pastBounds; } // Emulate the behaviour of the passed-in enumerator object System.Collections.IEnumerator.Current { get { if(!m_pastBounds) return m_current; else { if(m_atStart) m_enum.Reset(); return m_enum.Current; } } } public T Current { get { if(!m_pastBounds) return m_current; else { if(m_atStart) m_enum.Reset(); return m_enum.Current; } } } // Get the next element with bounds checking public T Lookahead { get { if(m_hasNext) return m_lookahead; else { if(m_atStart) m_enum.Reset(); return m_enum.Current; } } } // Get the last valid element (if any): don't do any bounds checking public T UncheckedCurrent { get { return m_current; } } // Get the next valid element (if any): don't do any bounds checking public T UncheckedLookahead { get { return m_lookahead; } } // Indicate whether the lookahead is valid public bool LookaheadAtEnd { get { return !m_hasNext; } } public void Dispose() { m_enum.Dispose(); } } public class TestComma { public static void Main(string[] args) { string[][] cases = new string[][] { new string[] { }, new string[] { "ABC" }, new string[] { "ABC", "DEF" }, new string[] { "ABC", "DEF", "G" }, new string[] { "ABC", "DEF", "G", "H" } }; foreach(string[] s in cases) Console.WriteLine(CommaJoin(s)); const int numElems = 4000; var sl = new List<string>(numElems); for(int i = 0; i < numElems; ++i) sl.Add(i.ToString()); var sw = new System.Diagnostics.Stopwatch(); sw.Start(); for(int j = 0; j < numElems; ++j) CommaJoin(sl); sw.Stop(); Console.WriteLine("{0} iterations of {0} elements in {1} (average: {2}/iteration)", numElems.ToString(), sw.Elapsed, new TimeSpan(sw.Elapsed.Ticks/numElems)); return; } public static string CommaJoin(IEnumerable<string> s) { StringBuilder sb = new StringBuilder("{"); using(var le = new LookaheadEnumerator<string>(s.GetEnumerator())) { if(le.MoveNext()) { sb.Append(le.UncheckedCurrent); while(le.MoveNext()) sb.Append((le.LookaheadAtEnd) ? " and " : ", ").Append(le.UncheckedCurrent); } } return sb.Append("}").ToString(); } }Anonymous
April 24, 2009
I've dropped my answer here; no back-tracking - purely additive: http://stackoverflow.com/questions/788535/eric-lipperts-challenge-comma-quibbling-best-answer/788561#788561Anonymous
April 24, 2009
Thank you for this interesting question, Eric. A variant that continues with the general theme of processing streams would be to consider the output string as a stream of characters and therefore implement a method: IEnumerable<char> GetCommaQuibbledStringCharacters(IEnumerable<string> inputStrings); This could be optimised for both space and time efficiency in interesting ways. As an aside, I think this kind of Q&A would be well suited to http://www.stackoverflow.com, where: GetCommaQuibbledStringCharacters(new[] { "people can refine their answers in-place (avoiding repeat postings)", "the code is automatically formatted and highlighted for you", "posts can be freely commented upon by everyone" }; Would you consider posting these sorts of puzzles there? This blog post has certainly proved to be inspiring to many people but the nature of a blog is such that it is better suited to the broadcast of thoughts and information rather than community involvement and open discussion.Anonymous
April 26, 2009
This version only needs a single pass through the collection, so performance should be pretty good. It uses the standard "foreach" statement rather than accessing the enumerator directly. Comments, tests etc omitted for brevity: I'm hoping that the code will speak for itself ;) public static string CommaQuibbling(IEnumerable<string> items) { Action<StringBuilder, string, string> append = (sb, delim, value) => { if (value != null) { if (sb.Length > 1) { sb.Append(delim); } sb.Append(value); } }; StringBuilder builder = new StringBuilder("{"); string lastItem = null; foreach (string item in items) { append(builder, ", ", lastItem); lastItem = item; } append(builder, " and ", lastItem); return builder.Append("}").ToString(); } Also posted to StackOverflow: http://stackoverflow.com/questions/788535/eric-lipperts-challenge-comma-quibbling-best-answer/790151#790151Anonymous
April 27, 2009
Oops. My previous answer didn't correctly handle any empty strings appearing at the beginning of the collection. Here's the fixed version: public static string CommaQuibbling(IEnumerable<string> items) { StringBuilder builder = new StringBuilder("{"); string lastItem = null; bool itemsAppended = false; Action<string, string> append = (delimiter, value) => { if (value != null) { if (itemsAppended) { builder.Append(delimiter); } builder.Append(value); itemsAppended = true; } }; foreach (string item in items) { append(", ", lastItem); lastItem = item; } append(" and ", lastItem); return builder.Append("}").ToString(); } Also posted to StackOverflow: http://stackoverflow.com/questions/788535/eric-lipperts-challenge-comma-quibbling-best-answer/790065#790065Anonymous
April 27, 2009
The comment has been removedAnonymous
April 27, 2009
After spending a tol of time coming up with the 'best' solution I decided that my first correct solution (proven by the Unit Tests) was the best one. This is assuming development efford is the most expensive resource. Unfortnately I no longer have my initial solution to show you because I wasted my time 'improving' it to look like some of the other posts above. I do still have my original UnitTest, meybe they will help you. using CommaSeparate; using Microsoft.VisualStudio.TestTools.UnitTesting; using System.Collections.Generic; namespace CommaSeparate.Test { /// <summary> ///This is a test class for Class1Test and is intended ///to contain all Class1Test Unit Tests ///</summary> [TestClass()] public class Class1Test { /// <summary> ///A test for Seperate ///</summary> [TestMethod()] public void Empty() { Test(new string[]{}, @"{}"); } [TestMethod()] public void Count1() { Test(new []{"ABC"}, @"{ABC}"); } [TestMethod()] public void Count2() { Test(new[] { "ABC", "DEF" }, @"{ABC AND DEF}"); } [TestMethod()] public void Count3() { Test(new[] { "ABC", "DEF", "GHI" }, @"{ABC, DEF AND GHI}"); } [TestMethod()] public void Count4() { Test(new[] { "ABC", "DEF", "GHI", "JKL"}, @"{ABC, DEF, GHI AND JKL}"); } public void Test(IEnumerable<string> input, string expectedResult) { string actual; actual = Class1.Seperate(input); Assert.AreEqual(expectedResult, actual); } } }Anonymous
April 28, 2009
static string method(IEnumerable<string> stringEnumeration) { StringBuilder sb = new StringBuilder(); sb.Append("{"); if (stringEnumeration.Count() == 0) { } else if (stringEnumeration.Count() == 1) { sb.Append(stringEnumeration.ElementAt(0)); } else { int i = 0; for (;i < stringEnumeration.Count()-2; i++) { sb.Append(stringEnumeration.ElementAt(i)+", "); } sb.Append(stringEnumeration.ElementAt(i)+" and "); sb.Append(stringEnumeration.ElementAt(i + 1)); } sb.Append("}"); string s = sb.ToString(); return s; }Anonymous
April 30, 2009
Code is Vb.net. Solution 1 -- A generic state machine: Class ListBuilder(Of T) Private mSb As New System.Text.StringBuilder("{") Private mP As T Private mState As Integer Public Sub Append(S As T) Select Case mState Case 0: mSb.Append(S) mState = 1 Case 1: mP = S mState = 2 Case 2 mSb.Append(", ") mSb.Append(mP) mP = S End Select End Sub Overrides Function ToString As String If mState > 1 Then mSb.Append(" and ") mSb.Append(mP) End If mSb.Append("}") Return mSb.ToString End Function Shared Function MakeList(List As IEnumerable(Of T)) As String Dim Lm As New ListBuilder(Of T) If List IsNot Nothing Then For Each S As T In List Lm.Append(S) Next End If Return Lm.ToString End Function End Class '*** 'Tests in the immediate window: ? ListBuilder(Of Char).MakeList("") ? ListBuilder(Of Char).MakeList("a") ? ListBuilder(Of Char).MakeList("ab") ? ListBuilder(Of Char).MakeList("abc") ? ListBuilder(Of Char).MakeList("abcd") Solution 2 -- a generic "bufferized" loop: Function MakeList(Of T)(List As IEnumerable(Of T)) As String Dim S As New System.Text.StringBuilder S.Append("{") If List IsNot Nothing Then Using E As IEnumerator(Of T) = List.GetEnumerator If E.MoveNext Then S.Append(E.Current) If E.MoveNext Then Dim P As T = E.Current Do While E.MoveNext S.Append(",") S.Append(P) P = E.Current Loop S.Append(" and ") S.Append(P) End if End If End Using End if S.Append("}") Return S.ToString End Function '*** 'Tests in the immediate window: ? MakeList("") ? MakeList("a") ? MakeList("ab") ? MakeList("abc") ? MakeList("abcd")Anonymous
May 06, 2009
I'm not sure how, but neither Scheme nor any other Lisp has been mentioned. Haskell made an appearance, so clearly there are some functional programmers here. This is similar to David and Claudiu's solutions, but I thought I should still mention it for language-list completeness: (define (oxford-list-join word-sequence) (let oxford* ((word-sequence word-sequence) (accum "{")) (cond ((null? word-sequence) (string-append accum "}")) ((null? (cdr word-sequence)) ;; last word gets nothing (oxford* '() (string-append accum (car word-sequence)))) ((null? (cddr word-sequence)) ;; second to last word postfix with " and " (oxford* (cdr word-sequence) (string-append accum (car word-sequence) " and "))) (else ;; all others join with ", " (oxford* (cdr word-sequence) (string-append accum (car word-sequence) ", ")))))) Sample input: (oxford-list-join '("first" "second" "third" "fouth")) Ran on Chicken Scheme, SISC, Petite Chez Scheme, and MzScheme. If you don't have Scheme, you can use the SISC interpreter online at http://sisc-scheme.org/sisc-online.phpAnonymous
May 12, 2009
It looks like I have arrived late to the party. This one is not very efficient, but it was fun to write. Plus, I expect extra credit for combining this solution with another of your blog posts! static void Main(string[] args) { //string[] input = {}; //string[] input = { "ABC" }; //string[] input = { "ABC", "DEF" }; string[] input = { "ABC", "DEF", "G", "H" }; var output = BuildStringSpecial(input); Console.WriteLine(output); } public static string BuildStringSpecial(IEnumerable<string> input) { int count = input.Count(); if (count == 0) return "{}"; Stack<string> seps = new Stack<string>(); seps.Push("}"); if (count > 1) seps.Push(" and "); if (count > 2) while (count > 2) { seps.Push(", "); count--; } StringBuilder body = new StringBuilder("{"); input.Zip(seps, (i, s) => { body.Append(i); body.Append(s); return ""; }).ToArray(); return body.ToString(); } Not much code. Not very efficient either, but it demonstrates your Zip operator nicely!Anonymous
May 14, 2009
The comment has been removedAnonymous
May 14, 2009
ALthough this doesn't perfectly satisfy the requirements because it doesn't wrap the result in { }, here's the tail recursive version: public static string Concat( IEnumerable<string> list ) { if (list.Count() == 0) return string.Empty; if (list.Count() == 1) return list.First(); return list.First() + (list.Count() > 2 ? ", " : " and ") + Concat( list.Skip( 1 ) ); }Anonymous
May 22, 2009
I went for one that's good with very large sets. This takes <40ms for 1000 strings and <400ms for 1,000,000 strings on my aging Pentium 4 workstation. (I have e-mailed this solution since I'm so late submitting, so Eric, you may skip this post). If you changed the return type to char[] I think this would even work beyond the limits of the String class. static string CreateLippertString(IEnumerable<string> strings) { char[] combinedString; char[] commaSeparator = new char[] { ',', ' ' }; char[] andSeparator = new char[] { ' ', 'A', 'N', 'D', ' ' }; int totalLength = 2; //'{' and '}' int numEntries = 0; int currentEntry = 0; int currentPosition = 0; int secondToLast; int last; int cbComma = 2 * sizeof(char); int cbAnd = 5 * sizeof(char); //calculate the sum of the lengths of the strings foreach (string s in strings) { totalLength += s.Length; ++numEntries; } //add to the total length the length of the constant characters if (numEntries >= 2) totalLength += 5; // " AND " if (numEntries > 2) totalLength += (2 * (numEntries - 2)); // ", " between items //setup some meta-variables to help later secondToLast = numEntries - 2; last = numEntries - 1; //allocate the memory for the combined string combinedString = new char[totalLength]; //set the first character to { combinedString[0] = '{'; currentPosition = 1; if (numEntries > 0) { //now copy each string into its place foreach (string s in strings) { Buffer.BlockCopy(s.ToCharArray(), 0, combinedString, currentPosition * sizeof(char), s.Length * sizeof(char)); currentPosition += s.Length; if (currentEntry == secondToLast) { Buffer.BlockCopy(andSeparator, 0, combinedString, currentPosition * sizeof(char), cbAnd); currentPosition += 5;//andSeparator.Length; } else if (currentEntry == last) { combinedString[currentPosition] = '}'; //set the last character to '}' break; //don't bother making that last call to the enumerator } else if (currentEntry < secondToLast) { Buffer.BlockCopy(commaSeparator, 0, combinedString, currentPosition * sizeof(char), cbComma); currentPosition += 2;//commaSeparator.Length; } ++currentEntry; } } else { //set the last character to '}' combinedString[1] = '}'; } return new string(combinedString); }Anonymous
July 01, 2009
Eric, take a look at my "pretty unmaintainable" solution :)) Anybody is free to use it for fun and for making money .. static string buildString(IEnumerable<string> mlist) { var count = mlist.Count(); var res = mlist.Select((mitem, index) => new { str = mitem + ((count - 1) == index ? "" : ( (count - 2) == index ? " and " : "," )) }); return res.Aggregate(new StringBuilder("{"), (builder, pitem) => builder.Append(pitem.str)).Append("}").ToString(); }Anonymous
July 01, 2009
Eric, can you make my contribution above readable? One way to see it, is select the text, copy it and past to a notepad...Then you will see the beauty of it ..... Here is a complete code to test the creation above: using System; using System.Collections.Generic; using System.Linq; using System.Text; namespace ErikLipertCHallenge { class Program { //here we test static void Main(string[] args) { var stringList = new string[][] { new string[] { "ABC", "DEF", "G", "H" }, new string[] { "ABC", "DEF" }, new string[] { "ABC" }, new string[] {}}; foreach (var mArray in stringList) { System.Console.WriteLine(buildString(mArray)); } } //actual implementation static string buildString(IEnumerable<string> mlist) { var count = mlist.Count(); var res = mlist.Select((mitem, index) => new { str = mitem + ((count - 1) == index ? "" : ( (count - 2) == index ? " and " : "," )) }); return res.Aggregate(new StringBuilder("{"), (builder, pitem) => builder.Append(pitem.str)).Append("}").ToString(); } } }Anonymous
October 26, 2009
I'm surprised to see that I'm the first one to use SelectMany: public static string Format(IEnumerable<string> list) { var formatElements = list.Reverse().SelectMany((s, i) => new List<string> { s, i==0 ? " and " : ", " }).Reverse().Skip(1); StringBuilder sb = new StringBuilder(); foreach (string fs in formatElements) sb.Append(fs); return "{" + sb + "}"; } The idea is that it's easy to remove an extra delimiter if it's an item of its own. If the input is [A, B, C], then the list SelectMany produces, will be: {C, " and ", B, ", ", A, ", "}, Reverse and Skip will then reorder the list correctly and remove the extra last element, which is ", " in this example. I won't claim that this code is friendly to the code maintainer or would perform very well. In production code I would go with the enumerator.MoveNext() approach.Anonymous
December 03, 2009
public static string Quibble(IEnumerable<string> quibbles) { StringBuilder b = new StringBuilder(); b.Append('{'); IEnumerator<string> quibbleMe = quibbles.GetEnumerator(); bool atLeastOneQuibbly = quibbleMe.MoveNext(); while (atLeastOneQuibbly) { b.Append(quibbleMe.Current); if (quibbleMe.MoveNext()) { string s = quibbleMe.Current; if (!quibbleMe.MoveNext()) { b.Append(" AND "); b.Append(s); break; } b.Append(','); b.Append(s); b.Append(','); } else break; } b.Append('}'); return b.ToString(); }Anonymous
December 04, 2009
Sorry about that last post....I had a brain f..., it obviously does not work. Following one should do the trick. Simple and easy IMHO. public static string CommaQuibble(IEnumerable<string> quibbleMe) { StringBuilder commaQuibbled = new StringBuilder(); IEnumerator<string> quibblies = quibbleMe.GetEnumerator(); commaQuibbled.Append('{'); quibblies.MoveNext(); if (quibblies.Current != null) { commaQuibbled.Append(quibblies.Current); quibblies.MoveNext(); while (quibblies.Current != null) { string nextQuibbly = quibblies.Current; if (quibblies.MoveNext()) commaQuibbled.Append(", "); else commaQuibbled.Append(" AND "); commaQuibbled.Append(nextQuibbly); } } commaQuibbled.Append('}'); return commaQuibbled.ToString(); }Anonymous
March 11, 2010
Couldn't resist this despite being a year late. I don't agree that "readability is terseness". I've seen too much terse but unreadable code which needed to be expanded to be understandable. To wit, here's my terse and unreadable version, comprised of a single line: static string Quibble1(IEnumerable<string> inList) { return "{" + (inList.Count() == 0 ? "" : inList.Count() == 1 ? inList.First() : string.Join(", ", inList.ToArray(), 0, inList.Count() - 1) + " and " + inList.Last()) + "}"; } But of all the versions I came up with, this one seems to have the best balance of terseness and readability: static string Quibble2(IEnumerable<string> inList) { string[] aList = inList.ToArray(); if (aList.Length <= 1) return "{" + (aList.Length == 0 ? "" : aList[0]) + "}"; string mainList = string.Join(", ", aList, 0, aList.Length - 1); return "{" + mainList + " and " + aList[aList.Length - 1] + "}"; } Except Eric's rules stipulated that only the methods of IEnumerable<> were allowed, so the above two solutions are invalid. Here's my first valid solution: static string Quibble3(IEnumerable<string> inList) { IEnumerator<string> e = inList.GetEnumerator(); string result = "{"; if (e.MoveNext()) result = result + e.Current; bool more = e.MoveNext(); while (more) { string current = e.Current; more = e.MoveNext(); result = result + (more ? ", " : " and ") + current; } return result + "}"; } Using StringBuilder would be more efficient for large lists, of course, but the structure's the same. And for fun, here's a valid version using recursion. But I wouldn't usually write code like this (I don't like inline assignment, or repurposing of variables): static string Quibble4(IEnumerable<string> inList) { IEnumerator<string> e = inList.GetEnumerator(); string s = "{"; if (e.MoveNext()) s = s + e.Current; return s + walk(e, e.MoveNext()); } static string walk(IEnumerator<string> E, bool more) { if (!more) return "}"; string current = E.Current; return ((more = E.MoveNext()) ? ", " : " and ") + current + walk(E, more); }Anonymous
March 11, 2010
I don't know what's got into me, but I just wrote an even more terrible version of the recursive one above. This is bordering on diabolical. static string Quibble666(IEnumerable<string> inList) { IEnumerator<string> e = inList.GetEnumerator(); bool more; return "{" + (e.MoveNext() ? e.Current : "") + Walk666(e, (more = e.MoveNext()), more ? e.Current : ""); } static string Walk666(IEnumerator<string> e, bool more, string current) { return more ? ((more = e.MoveNext()) ? ", " : " and ") + current + Walk666(e, more, more ? e.Current : "") : current + "}"; } (Anyone who actually codes like this will never work for me!)Anonymous
March 11, 2010
The comment has been removedAnonymous
September 09, 2010
My $0.02 (one-liner) static string Do(params string[] input) { return String.Concat( "{", input.Length > 2 ? String.Concat( String.Join(", ", input.Take(input.Length - 1)), " and ", input.Last()) : String.Join(" and ", input), "}"); } static void Main(string[] args) { Console.WriteLine(Do("")); // {} Console.WriteLine(Do("ABC")); // {ABC} Console.WriteLine(Do("ABC", "DEF")); // {ABC and DEF} Console.WriteLine(Do("ABC", "DEF", "G", "H")); // {ABC, DEF, G and H} }Anonymous
September 09, 2010
@David V. Corbin: Whut?? Understand NOTHING from your message.Anonymous
November 09, 2010
public class StringSequenceFormatter { public string Format(IEnumerable<string> sequence) { string commaList = RecursiveFormatter(sequence); return '{' + commaList + '}'; } private string RecursiveFormatter(IEnumerable<string> sequence) { if (sequence.Count() == 0) { return string.Empty; } else if (sequence.Count() == 1) { return sequence.First(); } else if (sequence.Count() == 2) { return sequence.First() + " and " + sequence.Last(); } else { return sequence.First() + ", " + RecursiveFormatter(sequence.Skip(1)); } } }Anonymous
April 11, 2011
Some of these solutions are so... odd that it makes me scratch my head wondering why. Here's my take, straight forward: public static string EnglishWordConcatination(this IEnumerable<string> source) { /* * * If the elements passed in are already an array, * use it. * / string[] sourceArray = source as string[]; int itemCount = 0; if (sourceArray == null) { / * * If the elements passed in are already a type-strict * collection of strings, use it. * / var sourceCollection = source as ICollection<string>; if (sourceCollection != null) { sourceArray = new string[itemCount = sourceCollection.Count]; sourceCollection.CopyTo(sourceArray, 0); } else { / * * Otherwise, build a new array assuming * a small working base set of elements. * / sourceArray = new string[2]; foreach (var element in source) { if (itemCount == sourceArray.Length) { string[] tempArray = new string[itemCount * 2]; sourceArray.CopyTo(tempArray, 0); sourceArray = tempArray; } sourceArray[itemCount++] = element; } } } else itemCount = sourceArray.Length; StringBuilder sb = new StringBuilder(); sb.Append('{'); bool first = true; for (int i = 0; i < itemCount; i++) { / * * The condition is exclusive, the last is Length - 1. * / bool last = (i == (itemCount - 1)); / * * Special conditions are the first and last since * the separator is appended prior to the current * element. * */ if (first) first = false; else if (last) sb.Append(" and "); else sb.Append(", "); sb.Append(sourceArray[i]); } sb.Append('}'); return sb.ToString(); }Anonymous
June 01, 2011
Just realized this was an old post, but a puzzle is a puzzle. This is what I came up with in C#. public string CommaQuibble(IEnumerable<string> data) { StringBuilder result = new StringBuilder(); int stringCount = data.Count(); int currentStringIndex = 1; foreach(string str in data) { if(currentStringIndex == 1) result.Append(str); else if(currentStringIndex < stringCount ) result.Append("," + str); else if(currentStringIndex == stringCount) result.Append(" and " + str); currentStringIndex++; } return string.Format("{{{0}}}", result.ToString()); }Anonymous
October 08, 2011
Without too many or nested if statements. Flat code. public static string CommaQuibbling(IEnumerable<string> items) { var result = new StringBuilder("{"); string separator, last; last = separator = ""; bool second = false; var iter = items.GetEnumerator(); if (iter.MoveNext()) result.Append(iter.Current); while (iter.MoveNext()) { second = true; result.Append(separator).Append(last); last = iter.Current; separator = ","; } result.Append(second ? " and ": separator).Append(last); return result.Append("}").ToString(); }