1

Is there a better—more functional, succinct, or elegant—way to write this? A reduce/fold function, perhaps?

var key = String.Join(String.Empty,
    new[] {
        keyRoot,
        controllerName,
        actionName
    }.Concat(
        from param in params
        select param.Key + param.Value
    )
);

The input is a few variables that are strings, as well as an enumerable of concatenated keys/values from a Dictionary<string, string>.

The output should be all of these strings concatenated.

FoobarisMaximus
  • 1,109
  • 3
  • 13
  • 18

6 Answers6

3

It sounds like you could use the LINQ Aggregate function:

Using LINQ to concatenate strings

Community
  • 1
  • 1
Jeff
  • 35,755
  • 15
  • 108
  • 220
  • Just to be clear: Aggregate is, in fact, a variety of the reduce/fold functions mentioned in the question. – C. A. McCann Jul 14 '11 at 16:44
  • The only problem with this approach is that the variables `keyRoot`, `controllerName`, and `actionName` are not in the same enumerable as the dictionary, so you'd still have to use a concat. – FoobarisMaximus Jul 14 '11 at 18:11
1

More readable to me would be something like this:

string key = string.Format("{0}{1}{2}{3}", 
                            keyRoot, 
                            controllerName, 
                            actionName, 
                            string.Join(string.Empty, parameters.Select( p =>  p.Key + p.Value)));

This might not be as "functional" but certainly as succinct and clear as I can come up with.

BrokenGlass
  • 158,293
  • 28
  • 286
  • 335
0

Here is a solution in one expressions using Aggregate (effectively a fold):

var key = params.Aggregate(new StringBuilder()
    .Append(keyRoot)
    .Append(controllerName)
    .Append(actionName),
    (sb, p) => sb.Append(p.Key).Append(p.Value))
    .ToString();
cdiggins
  • 17,602
  • 7
  • 105
  • 102
0

This doesn't improve it much...

var key = string.Concat(
   new[] {
    keyRoot,
    controllerName,
    actionName
   }.Concat(
       params.Select(kvp) => param.Key + param.Value)
   ).ToArray ()
);

This is 2 lines shorter if it doesn't have to be a single statement.

var list = new List<String> {
    keyRoot,
    controllerName,
    actionName
   };
list.AddRange (params.Select(kvp) => param.Key + param.Value));
var key = string.Concat(list.ToArray ());
agent-j
  • 27,335
  • 5
  • 52
  • 79
0

With an extension to StringBuilder:

public static class StringBuilderExtensions {

  public static StringBuilder AppendAll(this StringBuilder builder, IEnumerable<string> strings) {
    foreach (string s in strings) builder.Append(s);
    return builder;
  }

}

it gets rather short and efficient:

string key =
  new StringBuilder()
  .Append(keyRoot)
  .Append(controllerName)
  .Append(actionName)
  .AppendAll(parameters.Select(p => p.Key + p.Value))
  .ToString();

This will build the string without creating any intermediate arrays.

One thing to improve would be to avoid the intermittent strings p.Key + p.Value by adding the key and value directly to the StringBuilder, but then the code gets less reusable.

Another thing to improve would be to set the capacity of the StringBuilder, but then you would need to loop though the dictionary and add upp the length of the strings first.

(Note: I used parameters for the name of the dictionary instead of params, as that is a keyword.)

Guffa
  • 687,336
  • 108
  • 737
  • 1,005
0

I think for concatenating the sequence of all strings, your construct is already as functional as you can get.

Instead of using String.Join with an empty string, I'd probably use a StringBuilder together with a ForEach extenstion method like

public static class MyExtensions {
  public static void ForEach(this IEnumerable<T> enumerable, Action<T> action) {
    foreach (var entry in enumerable)
      action(entry);
  }
}

I also would define a local variable for the sequence like

var seq = new[] {
                   keyRoot,
                   controllerName,
                   actionName
          }.Concat(
           from param in params select param.Key + param.Value
          );
var sb = new StringBuilder();
seq.ForEach(s=>sb.Append(s));

Of course, using the Aggregate function would be more "functional", but it is not more readable in my opinion plus it has performance penalties, because you need to construct intermediate strings...

MartinStettner
  • 28,719
  • 15
  • 79
  • 106