Add StringValues operator+

[benaadams]

Background and Motivation

Currently if appending a value to a StringValues you need to use the static Concat method:

headers.ContentLanguage = StringValues.Concat(headers.ContentLanguage, requestCulture.UICulture.Name);

Overloading operator+ would allow it to be much cleaner

headers.ContentLanguage += requestCulture.UICulture.Name;

Proposed API

Please provide the specific public API signature diff that you are proposing. For example:

namespace Microsoft.Extensions.Primitives
{
    public readonly partial struct StringValues
    {
        public static StringValues operator +(StringValues left, StringValues right);
        public static StringValues operator +(string left, StringValues right);
        public static StringValues operator +(StringValues left, string right);
        public static StringValues operator +(string[] left, StringValues right);
        public static StringValues operator +(StringValues left, string[] right);
    }
}

Risks

Currently

headers.ContentLanguage += requestCulture.UICulture.Name;

Will compile; however it will implicitly convert the StringValues to a string add the two strings and then implictly convert back to a single value StringValue.

However; hopefully no-one is doing that as it will append the add in an incorrect way.

[ghost]

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and Motivation

Currently if appending a value to a StringValues you need to use the static Concat method:

headers.ContentLanguage = StringValues.Concat(headers.ContentLanguage, requestCulture.UICulture.Name);

Overloading operator+ would allow it to be much cleaner

headers.ContentLanguage += requestCulture.UICulture.Name;

Proposed API

Please provide the specific public API signature diff that you are proposing. For example:

namespace Microsoft.Extensions.Primitives
{
    public readonly partial struct StringValues
    {
        public static StringValues operator +(StringValues left, StringValues right);
        public static StringValues operator +(string left, StringValues right);
        public static StringValues operator +(StringValues left, string right);
        public static StringValues operator +(string[] left, StringValues right);
        public static StringValues operator +(StringValues left, string[] right);
    }
}

Risks

Currently

headers.ContentLanguage += requestCulture.UICulture.Name;

Will compile; however it will implicitly convert the StringValues to a string add the two strings and then implictly convert back to a single value StringValue.

However; hopeful no-one is doing that as it will append the add in an incorrect way.

Author:	benaadams
Assignees:	-
Labels:	api-suggestion, area-Extensions-Primitives, untriaged
Milestone:	-

[eerhardt]

Do we have a precedent for using the + operator to add a value to a collection? I don't think I've seen that in the BCL before.

[benaadams]

So the following

using System;
using Microsoft.Extensions.Primitives;

StringValues values = "hello";

values = StringValues.Concat(values, "world");

Console.WriteLine(values);

values += "how";

Console.WriteLine(values);

Complies and outputs

hello,world
hello,worldhow

Which is no use to anyone, whereas it would be better if it output

hello,world
hello,world,how

[eerhardt]

IMO, that's the issue with having implicit operators. They do unexpected things. I'm not sure if doing more unorthodox things is the right fix.

[aradalvand]

Currently, it's not at all clear exactly how a new value should be added to a StringValues instance. There are Append and Add methods that appear as suggestions in the editor, neither of which do what you would expect them to.

Add throws a NotSupportedException, and Append is a LINQ method that is similar to StringValues.Concat and returns a new instance. It's very easy to fall into the trap of thinking the following line of code is actually doing something — when in reality it isn't:

Response.Headers.Vary.Append(HeaderNames.Referer);

The API in this regard is in dire need of some refinement. There are even comments in the ASP.NET Core codebase itself pointing out these very same unexpected quirks when it comes to adding things to StringValues, which goes to show how non-obvious its API really is.