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

Allowing outer constructors for abstract types as a factory idiom #8159

Closed
mcg1969 opened this issue Aug 27, 2014 · 7 comments
Closed

Allowing outer constructors for abstract types as a factory idiom #8159

mcg1969 opened this issue Aug 27, 2014 · 7 comments

Comments

@mcg1969
Copy link

mcg1969 commented Aug 27, 2014

I'm taking a risk here that my GitHub search fu is lacking. But just to show I tried, this issue seems related to #759.

Has any thought been given to allowing abstract classes to have outer constructor methods? I know, I know: why would you do that, given that an abstract class cannot be instantiated? Well, it could return subclass objects instead; that is, it could be a factory that instantiates different concrete class types in a data-dependent fashion. It is already the case that inner or outer constructors for non-abstract classes are free to return any object they wish (to be clear: that does not strike me as a good idea).

Here is a hypothetical, and arguably dumb, example.

abstract Scalar{T<:Number}
immutable Zero{T<:Number} <: Scalar{T}
end
immutable One{T<:Number} <: Scalar{T}
end
immutable NotZeroOrOne{T<:Number} <: Scalar{T}
    value::T
    function NotZeroOrOne( v::T )
        v == 0 && return Zero{T}()
        v == 1 && return One{T}()
        new(v)
    end
end
NotZeroOrOne( v::Number ) = NotZeroOrOne{typeof(v)}( v )
function Scalar{T<:Number}( v::T )
   v == 0 && return Zero{T}()
   v == 1 && return One{T}()
   NotZeroOrOne{T}(v)
end
value{T}(::Zero{T}) = zero(T)
value{T}(::One{T}) = one(T)
value{T}(x::NotZeroOrOne{T}) = x.value

I've added an inner constructor for the NotZeroOrOne class that ensures that the zero or one cases are appropriately farmed out to the proper class---just so you can see that it's possible. This returns the following error:

invalid method definition: not a generic function
while loading In[1], in expression starting on line 15

Of course, all I have to do to fix this is, say, change the name of the factory function to, say, scalar. With that small change, both the NotZeroOrOne constructor and the scalar function behave identically.

In [2]: NotZeroOrOne(1)
Out[2]: One{Int64}()
In [3]: scalar(1)
Out[3]: One{Int64}()

I readily admit that there is no missing functionality here. I also recognize that for non-abstract classes, it's bad form to return anything but an instance of the class (or an exception). But it does seem like it would be a nice convenience for abstract classes, and one with a reasonable interpretation.

@johnmyleswhite
Copy link
Member

I've done this in the past. One problem with this idea is that you end up with type-unstable functions, which thwart the compiler.

@mcg1969
Copy link
Author

mcg1969 commented Aug 27, 2014

Good point, and a specific reason why returning other objects from a concrete class constructor is a bad thing to do. And yet, by tying this factory function to the abstract class it generates for, you're offering the compiler a clue as to the function's intent. It could be readily verified that the return type is a Union of several concrete subclasses of the same abstract class; and perhaps this information could be useful.

@Jutho
Copy link
Contributor

Jutho commented Aug 27, 2014

It would have been easy to cook up an example where the abstract constructor is not type unstable, i.e. if the type of the concrete object being constructed only only depends on the type of the arguments given to the abstract constructor.

@mcg1969
Copy link
Author

mcg1969 commented Aug 27, 2014

And to be clear, sometimes type instability is a price one may willing to pay. The objects I'm actually considering using this with are not likely to be the source of performance bottlenecks.

@Jutho
Copy link
Contributor

Jutho commented Aug 27, 2014

That is to say, +1 for this proposal, I would like to see this functionality, but it was already mentioned in #1470 by @stevengj , as part of the call overload suggestion.

@mcg1969
Copy link
Author

mcg1969 commented Aug 27, 2014

Ah, very good, thank you for the pointer. I took a look at that issue but it was pretty thick so I missed the clear similarity.

@JeffBezanson
Copy link
Sponsor Member

near-dup of #3427, also planned as part of #1470.

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

4 participants