Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Shorter syntax for simple structures that encapsulate a single value. #3170

Closed
thargol1 opened this issue Feb 7, 2020 · 8 comments

Comments

@thargol1
Copy link

thargol1 commented Feb 7, 2020

Inspired by some epic bugs in our codebase, I created 'simple datatypes' for stuff like 'Email', 'Emplid', 'Usercode' etc.

It prevents bugs like:

Email mail;
Usercode user = "1234567@domain.com";

email = user; // <-- generates compile error

old code:

string mail;
string user = "1234567@domain.com";

email = user; // <-- compiles fine, but is a bug.

My datatypes look like tthe code below... a lot of standard boiler plate code. I don't want to use classes because of the extra memory they use and they require more changes to existing code.

This is a complete example of an email datatype:

public readonly struct Email : IEquatable<Email>
{
	public readonly string? Value;

	public Email(string? value) => Value = value;

	public bool IsValid => Validate(Value);

	public static bool Validate(string? email)
	{
		// code ommited 
	}

	public static bool TryParse(string value, out Email outValue)
	{
		outValue = new Email(value);
		return outValue.IsValid;
	}

	public bool Equals(Email other) => other.Value == Value;
	public override bool Equals(object obj) => (obj is Email v && Equals(v)) || (obj is null && Value is null);
	public override int GetHashCode() => 27436719 + (Value ?? String.Empty).GetHashCode();
	public override string ToString() => Value ?? string.Empty;

	public static bool operator ==(Email left, Email right) => left.Equals(right);
	public static bool operator !=(Email left, Email right) => !(left == right);

	public static implicit operator string?(Email v) => v.Value;
	public static implicit operator Email(string? v) => new Email(v);
}

The problems with this code:

  • It's a lot of code for a simple problem.
  • I have to write converters for NewtonSoft, Dapper, EF core or whatever.

Converter code for NewtonSoft

[JsonConverter(typeof(EmailConverter))]
public readonly struct Email : IEquatable<Email>
{
// ommitted
}

public class EmailConverter : JsonConverter
{
	public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) =>
		writer.WriteValue(((Email)value).Value);

	public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer) =>
		new Email((string)reader.Value);

	public override bool CanConvert(Type objectType) => objectType == typeof(Email);
}

However if this became a feature in the C# language the creators of NewtonSoft, Dapper, EF core could build standard support for it.

Syntax proposal:

datatype Email : string;

This generates the code above without the validation and TryParse

datatype Email : string
{
	public static bool Validate(string value) =>
	{
		// Email validation code ommitted
	}
}

This genates the code above including validation and TryParse.

An example I wish I could use right now:

datatype Emplid : int
{
	public static bool Validate(int value) => value > 10000 && value <= 99999;
	public override string ToString() => Value.ToString("D5");
}

Pro's of using this kind of simple datatypes:

  • It makes code more readable.
  • It prevents code errors at compile time.
  • Memory requirements almost stay the same, because it's a structure.

Cons:

  • To make this a viable language construct third parties like Dapper, EF core, NewtonSoft and other have to add support for this feature otherwise it's generates more work than it reduces.
  • Because third parties have to be able to detect this types I don't think this can be implemented as a compiler-only feature. I think reflection should also be updated.
@orthoxerox
Copy link

See also #1695 #410

@DaZombieKiller
Copy link
Contributor

Source generators could also be another solution to this problem.

@dsaf
Copy link

dsaf commented Feb 8, 2020

Very good idea, see also #259 #1170.
Related but not similar #3137.

@alrz
Copy link
Member

alrz commented Feb 10, 2020

This is just a record with a single member? data struct Email(string Value);

@thargol1
Copy link
Author

This is just a record with a single member? data struct Email(string Value);

Yes this looks like a record with a single member, but a record is a class and not a struct (#2699). In order to migrate existing code, converting existing integers and strings is easier with structs, just change the type and you're done. With classes you have to add new-statements etc. And classes are just slower in performance (not a lot).

Because this is a single member struct, a lot of boilerplate code can be generated, like automatic type conversions from and to the underlying type, which in my opinion is a must to facilitate migrating existing code.

I'm still reading #1695, but it looks like I can close this issue.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Feb 10, 2020

This feels like a space for an IDE feature. Feels like we could easily add a 'create wrapper struct' feature where we generate all that conversion logic and we allow you to specify what you want to expose from the wrapper.

@AartBluestoke
Copy link
Contributor

@CyrusNajmabadi if everybody implements their own version of this, then there can't be any third party support for eg: serialisation for almost-a-string Email(string e) (as mentioned in the 'cons' of the FP)

This is enough of an issue that visual studio always shows variable names on the calee of a function just to provide you the visual hint to not put the email into the username, but this would provide a Guarantee with a few extra lines of code.

The only question IMO is "is this extra complexity in the language worth the bugs it will save, via bringing extra context based type-safety to use of int,string, and bool"

@thargol1
Copy link
Author

I'm closing this issue and will add suggestions to #3137. It looks very similar but it's all in the details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants