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] Allow constructors to invoke methods that modify readonly fields #1871

Closed
bryancrosby opened this issue Sep 18, 2018 · 7 comments
Closed

Comments

@bryancrosby
Copy link

bryancrosby commented Sep 18, 2018

Sometimes when initializing a class, the setting of readonly variables can make constructors lengthy or hamper readability.

This proposal aims to improve the readability of constructor code by introducing a modifier keyword to a method called initonly.

Requirements

  • May not be called from any code that is not invoked directly from the constructor.
  • May not modify any fields that are not explicitly marked as readonly.
  • Should not be allowed to be invoked from an inherited class.
  • Is implicitly private
  • Can call as many methods as needed, as long as each method invoked is marked initonly

Current workarounds

There aren't any really great workarounds right now that I can think of, except one.

out params

You can specify parameters, but this looks kind of hackish.

public class Machine
{
	private readonly int instanceValue;

	public Machine(int minutes)
	{
		ConfigureVariables(out instanceValue, minutes);
	}

	private static void ConfigureVariables(out int assignToThisField, int fromThisValue)
	{
		assignToThisField = fromThisValue;
	}
}

Samples

Single method invocation

public class Machine
{
	private readonly int timeOnline;
	private readonly long secondsUntilExpiration;
	private readonly string name;

	public Machine(Func<Configuration> configFunc)
	{
		SetConfiguration(configFunc());
	}

	private initonly Configuration(Configuration config)
	{
		timeOnline = config.TimeOnline;
		name = config.Name ?? "Default Machine";
	}
}

Multiple method invocations

public class Machine
{
	private readonly int timeOnline;
	private readonly long secondsUntilExpiration;
	private readonly string name;

	public Machine(Func<Configuration> configFunc)
	{
		SetConfiguration(configFunc());
	}

	private initonly SetConfiguration(Configuration config)
	{
		timeOnline = config.TimeOnline;
		name = config.Name ?? "Default Machine";
		AdditionalStuff(config);
	}

	private initonly AdditionalStuff(Configuration config)
	{
		timeOnline = config.Online > 100 ? 100 : 5;
		name = config.Name ?? "Default Machine";
	}
}

Advantages

  • Better code readability
  • Specific enough that it's reasonable to assume the method is only ever invoked through the constructor.

Disadvantages

  • Could raise questions of class design and responsibilities (not necessarily true but may raise a smell)
@jnm2
Copy link
Contributor

jnm2 commented Sep 18, 2018

Same as #348 and #1296?

@Austin-bryan
Copy link

All methods are implicitly private, so it should instead be unable to be public.

@theunrepentantgeek
Copy link

I believe it's been previously mentioned that the complexity of the required flow analysis for this is likely to be prohibitive - especially once you start considering lambdas (esp variable capturing), async/await, versioning, and so on.

FWIW, I've used tuples as workarounds to good effect, e.g.:

public class Machine
{
    private readonly int _value;
    private readonly string _name;

    public Machine(int minutes)
    {
        (_value, _name) = Configure(minutes);
    }

    private static (int value, string name) Configure(int minutes)
    {
        return (minutes, minutes.ToString());
    }
}

@CyrusNajmabadi
Copy link
Member

All methods are implicitly private, so it should instead be unable to be public.

Private methods can still be accessed by other types. That seems... dangerous :)

@HaloFour
Copy link
Contributor

I imagine a possible implementation for a feature like this would be that the initonly method would "capture" any written fields as ref parameters. When calling the method from a constructor the readonly fields would be passed by ref to those parameters, which will allow them to be written.

For example:

public class Machine
{
	private readonly int timeOnline;
	private readonly long secondsUntilExpiration;
	private readonly string name;

	public Machine(Func<Configuration> configFunc)
	{
		SetConfiguration(configFunc());
	}

	initonly void SetConfiguration(Configuration config)
	{
		timeOnline = config.TimeOnline;
		name = config.Name ?? "Default Machine";
		AdditionalStuff(config);
	}

	initonly void AdditionalStuff(Configuration config)
	{
		timeOnline = config.Online > 100 ? 100 : 5;
		name = config.Name ?? "Default Machine";
	}
}

could be "compiled down" to:

public class Machine
{
	private readonly int timeOnline;
	private readonly long secondsUntilExpiration;
	private readonly string name;

	public Machine(Func<Configuration> configFunc)
	{
		SetConfiguration(configFunc(), ref timeOnline, ref name);
	}

	private void SetConfiguration(Configuration config, ref int timeOnline, ref string name)
	{
		timeOnline = config.TimeOnline;
		name = config.Name ?? "Default Machine";
		AdditionalStuff(config, ref timeOnline, ref name);
	}

	private void AdditionalStuff(Configuration config, ref int timeOnline, ref string name)
	{
		timeOnline = config.Online > 100 ? 100 : 5;
		name = config.Name ?? "Default Machine";
	}
}

I've taken a few liberties with the proposed syntax. I've changed initonly to act as an accessibility modifier rather than as a return type. The actual accessibility modifier would always be private. This also allows the method to return a value.

This is also completely safe/verifiable as the initonly methods aren't actually writing to the fields but only to references to what might be those fields. The question is whether or not encoding this pattern into syntax candy is worthwhile.

@DavidArno
Copy link

DavidArno commented Sep 19, 2018

As a fan of pure methods, I really dislike features like this as they encourage what I see as bad practice: accessing fields directly from within arbitrary methods. I appreciate that others will disagree with this stance, but this is my view on it and I strongly believe it's the right view too.

So I would tackle this issue completely differently:

public class Machine
{
    private readonly int _timeOnline;
    private readonly long _secondsUntilExpiration;
    private readonly string _name;

    public Machine(Func<Configuration> configFunc)
    {
        (_timeOnline, _name) = SetConfiguration(configFunc());
    }

    private static (int timeonline, string name) SetConfiguration(Configuration config)
    {
        var timeOnline = config.TimeOnline;
        var name = config.Name ?? "Default Machine";
        return (timeOnline, name);
    }
}

That way, constructors can call other methods, and fields can be marked as readonly and updated via the use of those methods, all without having to come up with some solution to allowing methods to write to readonly fields. No language change needed, just a change in how folk use the language.

@YairHalberstadt
Copy link
Contributor

Closing as duplicate of #348

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

8 participants