Skip to content

Consider the binding space, including whether BinderBase<T>, IValueSource, and IValueDescriptor are needed in the public API for usage beyond SetHandler #1918

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

Closed
Tracked by #1891
jonsequitur opened this issue Nov 2, 2022 · 0 comments · Fixed by #2124

Comments

@jonsequitur
Copy link
Contributor

jonsequitur commented Nov 2, 2022

namespace System.CommandLine.Binding
{ 
    // This is the only hit of BinderBase in this proposal.  Is it needed?
    // The "Base" suffix is recommended against unless it's a very niche carve-out from a class that uses the better name.
    // Basically, if more than 30 people in the whole would would ever need to write it in their code you need a better name.
    public abstract class BinderBase<T> : IValueDescriptor<T>, IValueDescriptor, IValueSource 
    {
        protected T GetBoundValue(BindingContext bindingContext);
    }

    // In general, interfaces should only define capabilities, not actual structure.
    // If at all possible, this should be turned into an abstract class.
    // Note that interfaces can basically never change (we have never successfully added a DIM as a way to expan an interface).
    // So with this interface there's no proven way to add, e.g. T GetDefaultValue<T>(Func<T> defaultValue) in a later version
    // in a way that avoids boxing values into object.
    public interface IValueDescriptor 
    {
        bool HasDefaultValue { get; }
        string ValueName { get; }
        Type ValueType { get; }
        object GetDefaultValue();
    }
 
    // Interfaces with no members are effectively prohibited.
    public interface IValueDescriptor<out T> : IValueDescriptor 
    {
    }
 
    // This doesn't look like a capability, so a base class is very strongly preferred over an interface.
    // (We really don't like interfaces)
    public interface IValueSource 
    {
        // We really don't like ref.  Is this actually C#'s `out`?
        bool TryGetValue(IValueDescriptor valueDescriptor, BindingContext bindingContext, ref object& boundValue);
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants