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

Abstract Properties #4669

Closed
denvned opened this issue Sep 6, 2015 · 37 comments
Closed

Abstract Properties #4669

denvned opened this issue Sep 6, 2015 · 37 comments
Assignees
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@denvned
Copy link
Contributor

denvned commented Sep 6, 2015

This is a proposal to extend #3578 to allow abstract properties in abstract classes. So the following should be legal:

abstract class A {
  abstract p;
}
class B extends A {
  get p() {...}
  set p(v) {...}
}
@danquirk danquirk added Suggestion An idea for TypeScript In Discussion Not yet reached consensus Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. and removed In Discussion Not yet reached consensus labels Sep 8, 2015
@Pajn
Copy link

Pajn commented Sep 27, 2015

+1

Also abstract getters should be full-fillable by properties:

abstract class A {
  abstract get p();
}
class B extends A {
  p;
}

This is a pretty common pattern in Dart when the abstract class just need some values by it's implementers.

@mhegazy mhegazy added Declined The issue was declined as something which matches the TypeScript vision and removed Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Oct 2, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Oct 2, 2015

declaring the property as abstract does not change anything different from just declaring it. the type is going to always have the property.

As for getters and setters., the type system does not differentiate between getter/setter and a property declaration.

@mhegazy mhegazy closed this as completed Oct 2, 2015
@denvned
Copy link
Contributor Author

denvned commented Oct 4, 2015

@mhegazy Actually, there is an important difference. Using an abstract property I could specify that the property must be specialized (usually with getters and/or setters) in a derived class, and the compiler should rise an error if it's not specialized.

Please reconsider closing this proposal.

@raveclassic
Copy link

@mhegazy I totally agree with @denvned about enforcing getter declaration in child classes.

@kbtz
Copy link

kbtz commented Oct 20, 2015

The lack of abstract properties allows child classes to simply ignore some important properties in my contracts.

Please reopen this for further discussion...

@craigbroadman
Copy link

+1

Especially as lambda/arrow functions are actually properties as I found out when I was trying to implement the following functionality but it wouldn't compile due to....

"Class 'Base' defines instance member function 'def', but extended class 'Concrete' defines it as instance member property"...

abstract class Base {
    abstract abc(): void;
    abstract def(): void;
}

class Concrete extends Base {
    private setting: boolean;

    public abc(): void  {
        this.setting = true;
    }

    public def = (): void => {
        this.setting = false;
    }
}
@raveclassic
Copy link

Please reopen for discussion!

Here is a simple example describing the problem:

abstract class Foo {
    abstract getName(); //works like a charm
    abstract setName(value);

    /* but it's better to use properties:
    abstract get name();
    abstract set name(value);
    */

    /* I can define properties */
    private _name = '';
    get name() {
        return this._name;
    }
    set name(value) {
        this.name = value; 
    }
    /* but cannot force Bar class to implement them */
}

class Bar extends Foo {
    //complains only about getName and setName
}

Compiler complains about undefined getName and setName but I can't define property contract.
But hey, JS and TS both support getters/setters - why should I use Java-style methods instead of properties?

@sirudog
Copy link

sirudog commented Oct 30, 2015

+1
I agree, this is needed!
I just ran into this. Using abstract getters/setters actually can be done if you ignore this error:
error TS1242: 'abstract' modifier can only appear on a class or method declaration.
but would be nice if the compiler checked whether the property was implemented in subclasses.

@tehsenaus
Copy link

+1
Lack of abstract getters/setters is an oversight imo

@AJamesPhillips
Copy link

@raveclassic I'm interested in your request but I don't yet understand why the class inheriting from the abstract class needs it's implementation of an attribute to be enforced using getters and setters. Should it not be the responsibility of the child class how it implements an attribute? In your example, what is missing with:

abstract class Foo {
    name: string;
}

class Bar extends Foo {
    get name() {return "..."};
    set name(v) {"..."};
}

If someone implements another class without specifying the attribute at all, or by redeclaring it as a normal attribute:

class Baz extends Foo {}
class Fizz extends Foo {
    name: string;
}

...in all three classes (that inherit from the abstract class) they have a valid and functioning name attribute. What is missing? And if the answer is: "Anyone inheriting from this particular abstract class should be forced to implement getters and setters", then the following questions is "Why should anyone inheriting from this particular abstract class should be forced to implement an attribute in a particular way, either with, or without, getters and setters?":

@AJamesPhillips
Copy link

(I unfortunately haven't yet grasped with @craigbroadman is referring to but) perhaps @vagaroso, @tehsenaus, @sirudog or @denvned could also help illuminate what's missing from the examples I have shown in my previous comment please?

@raveclassic
Copy link

@AJamesPhillips The main reason in using abstract getters is that if I leave name attribute uninitialized in Foo class, I'll get undefined when accessing the value and possible runtime errors related to this. I want all access-related issues to be checked by compiler.
I believe I should provide a bit more concrete example:

export enum DataEventType {
    TRADE,
    QUOTE
}

export type DataTO = TradeTO | QuoteTO;

export abstract class DataEvent {
    protected _instrument:Instrument;
    protected _data:DataTO;

    get instrument():Instrument {
        return this._instrument;
    }

    get type():DataEventType {
        return this.getType();
    }

    /**
     * todo: make abstract when typescript supports abstract getters
     */
    get data():DataTO {
        return this._data;
    }

    constructor(data:DataTO) {
        this._data = data;
        this._instrument = this.createInstrument(data);
    }

    protected abstract getType():DataEventType;

    protected abstract createInstrument(data:DataTO):Instrument;
}

export class TradeDataEvent extends DataEvent {
    get data():TradeTO {
        return this._data as TradeTO;
    }

    protected getType() {
        return DataEventType.TRADE;
    }

    protected createInstrument(data:TradeTO):Instrument {
        return Instrument.fromString(data.symbol);
    }
}

export class QuoteDataEvent extends DataEvent {
    get data():QuoteTO {
        return this._data as QuoteTO;
    }

    protected getType() {
        return DataEventType.QUOTE;
    }

    protected createInstrument(data:QuoteTO):Instrument {
        return Instrument.fromString(data.symbol);
    }
}

In the example above I need all DataEvents (TradeDataEvent, QuoteDataEvent) to be forced to strictly specify the return type of data field. The same applies to getType with the only difference that here it is implemented as a method which is called in an inherited getter type, but it realy seems that it would be much more preferable to just use an abstract get type().

@AJamesPhillips
Copy link

Hey @raveclassic thanks for the clarification. I tried to reduce the example to highlight the essence of what you wanted to achieve so let me know if I've missed a crucial requirement / component. As you can see you're opening yourself up to errors using casting:

class DataTO {};
class TradeTO extends DataTO {};
class QuoteTO extends DataTO {
  quoteNum(): number {return 4};
};

abstract class DataEvent {
    protected _data: DataTO;

    /**
     * todo: make abstract when typescript supports abstract getters
     */
    get data(): DataTO {
        return this._data;
    }

    constructor(data: DataTO) {
        this._data = data;
    }
}

class TradeDataEvent extends DataEvent {
    get data(): TradeTO {
        return this._data as TradeTO;  // avoid casting if possible
    }
}

class QuoteDataEvent extends DataEvent {
    get data(): QuoteTO {
        return this._data as QuoteTO;  // avoid casting if possible
    }
}

var c = new QuoteDataEvent(new TradeTO());
console.log(c.data.quoteNum());  // run time error

I guessed at what you're trying to achieve, again correct me if I'm wrong, but I think a better pattern for this would be the following. Whilst I was trying to produce this I did think at one point that abstract setter or getter would again be useful, in this example because the setter does something each time with the DataTO value, but as you can see generics are the way forward!

class DataTO {
    revenue: number;
};
class TradeTO extends DataTO {};
class QuoteTO extends DataTO {
    quoteNum(): number {return 4};
};

abstract class DataEvent<DataTOGenericType extends DataTO> {
    private _data: DataTOGenericType;
    get data(): DataTOGenericType { return this._data};
    set data(v: DataTOGenericType) {
        // do something with v each time and in all subclasses
        v.revenue += 0;
        this._data = v;
    };

    constructor(data: DataTOGenericType) {
        this.data = data;
    }
}

class TradeDataEvent extends DataEvent<TradeTO> {}

class QuoteDataEvent extends DataEvent<QuoteTO> {}

var c = new QuoteDataEvent(new TradeTO());  // compile time error
var d = new TradeDataEvent(new TradeTO());
console.log(d.data.quoteNum());  // compile time error

Hope that helps.

@raveclassic
Copy link

@AJamesPhillips Interesting, somehow I forgot about generic type guards in class declarations.. =)
Yes, your solution perfectly fits even if DataTOs are interfaces.
Thanks a lot!

UPD: As for @craigbroadman example, I can imagine a very common situation when using React and instance-bound handlers:

abstract class Popup extends React.Component {
    abstract onClose(): void; //cannot use 'abstract onClose: () => void`

    render() {
        return (
            <button onClick={this.onClose}/>
        );
    }
}

class CustomPopup extends Popup {
    _onClose() { //correct but unbound

    }

    onClose = () => { //this is an instance-bound handler
        //here the compiler complains aboud difference in onClose declarations
        //trying to access 'this'
    }
}

One solution is to always bind handler either when passing to inner components: onClick={this.onClick.bind(this)} or in constructor this.onClose = this.onClose.bind(this); but both ways are inconvenient.
The other - to use methods declared as properties, but here compiler either complains about difference in method declaration or about inability to mark onClose property as abstract.

@tehsenaus
Copy link

@AJamesPhillips
Isn't the whole point of abstract to force an implementation of some kind? Of course it's up to the subclass to decide how to implement; abstract properties don't preclude this. Example:

abstract class Base {
    abstract x: string;
}
class A extends Base {
    x: string;
}
class B extends Base {
    get x(): string {
        ...
    }
}
class C extends Base {
    // Error
}

The issue is at the moment there is no way to express that a property must be implemented in some way by a subclass. I'm forced to do things like this:

abstract class Base {
    get x(): string {
        throw "x: not implemented!";
    }
}

Yuck!

@AJamesPhillips
Copy link

Yes @tehsenaus and I don't have any answer to that.

Do you have a concrete example we could look at? One where you are prefably exporting an abstract base class that you want the software engineer using that class to know at compile time that they need to give an x property? And that the abstract class having the following is not good enough and if so why:

abstract class Base {
    x: string;
}

Why should the abstract class enforce on its descendants the necessity to define a property when it could just do the following (sorry tone of this sounds a bit aggressive... Not meant to be I assure you :) )

@tehsenaus
Copy link

I can't give a concrete example without sharing proprietary source... but let's say I'm implementing the 'template method pattern', where the abstract class needs to read a property from a subclass. I could use a getter method... but why? This isn't Java!

@AJamesPhillips
Copy link

No obviously that's fine @tehsenaus, good example though... and what is undesirable about specifying the property in the abstract base (sorry if I'm being stupid and have just missed the point of 'template method pattern').

@raveclassic
Copy link

@AJamesPhillips I think that template method means that a subclass has to implement a method that is used in abstract class (for example in base render method) and it is assumed that this method cannot return any default value for being abstract. If it could, then just specifying this default value in abstract base would be enough.
The point here in favor of abstract properties (and I mentioned it earlier) is that it is not Java and devs should not be forced to use Java-style methods to access a simple value. I mean that I don't need pairs of getValue/setValue per each field. Moreover, everything related to runtime checks and exception throwing is undesired - we have static checking for that.

Also, what do you think about instance-bound handlers which are described in my previous message update? It seems like I've updated it too late and you haven't noticed that. =)

@AJamesPhillips
Copy link

Yes @raveclassic I did respond as soon as I was notified :) Re "instance-bound handlers" I don't understand why the abstract class Popup requires its subclasses to return void from onClose when it needs to return an object (i.e. hash / dictionary)?

Re: "template method",

a subclass has to implement a method that is used in abstract class

That's correct, but we're referring to properties. As mhegazy said earlier:

declaring the property as abstract does not change anything different from just declaring it. the type is going to always have the property.

As for getters and setters., the type system does not differentiate between getter/setter and a property declaration.

@AJamesPhillips
Copy link

@tehsenaus Sorry I don't understand. I agree that:

  • an abstract class should enforce its subclasses to implement abstract methods.

I also think that:

  • an abstract class declaring any property should not enforce a subclass to implement a get or set method as what implementation is required?

Again even a partially concrete use case would be very useful to understand what problem you are trying to solve and how you think abstract properties would help. Is it that you want the abstract class to force the subclass to initialise the property? Or provide some particular functionality? Or something else?

@Pajn
Copy link

Pajn commented Dec 11, 2015

Dart have abstract properties and uses them quite a lot in the standard library in a very nice way.
For example List which is an abstract class and have abstract getters for things like length, reversed and similar things that classes implementing a List need to implement.
https://github.com/dart-lang/sdk/blob/master/sdk/lib/core/list.dart

I can't see how a language that have getters and setters can justify abstract methods but not abstract properties. They are the same thing just with a nicer syntax. If you don't think abstract properties are needed you should think that abstract methods are equally needless.

@AJamesPhillips
Copy link

"abstract properties are the same thing just with a nicer syntax." @Pajn I agree. And of course abstract methods are valuable, they force subclasses to implement them. In a comment just before yours I have used one :)

Thanks for the link to the dart library. So is it only a case for brevity? i.e. it would allow you to condense this:

abstract class List {
    get length(): number { return this.getLength(); };
    set length(length: number) { this.setLength(length); };
    protected abstract getLength(): number;
    protected abstract setLength(length: number);
}

class SubList extends List {
    private _length: number;
    protected getLength() { return this._length; }
    protected setLength(length: number) { this._length = length; }
}

class HalfSubList extends List {
    private _length: number;
    protected getLength() { return this._length/2; }
    protected setLength(length: number) { this._length = length; }
}

To this:

abstract class List {
    abstract length: number;
}

class SubList extends List {
    length: number;
}

class HalfSubList extends List {
    private _length: number;
    get length() { return this._length/2; }
    set length(length: number) { this._length = length; }
}

Or is there a pattern in TypeScript that you just can't achieve without abstract properties? I've tried creating one but failed so far. I can see how it saves you several lines of code but personally I don't mind the Java like getters and setters, I'm primarily interested in patterns you just can't currently implement with TypeScript.

@Pajn
Copy link

Pajn commented Dec 12, 2015

@AJamesPhillips

And of course abstract methods are valuable, they force subclasses to implement them.

Just as abstract properties forces subclasses to implement them. They are the same thing.

So is it only a case for brevity?

Mostly, however I would argue that it's about intention as well. A property should never have a side effect, methods might. Mixing Java style getters with good getters makes the code more confusing.

Or is there a pattern in TypeScript that you just can't achieve without abstract properties?

We could probably think of some strange edge case where some library expect a good getter but I would say no. You can manage just fine without them. But you can manage just fine without abstract methods as well by having an abstract class implement an interface. But that doesn't mean that it's the clearest way to express yourself.

I would totally understand declining abstract properties if there were no abstract methods but when you already have the concept of abstract things I can't understand why not all methods can be abstract. It makes the language harder to learn and understand when you have to remember all special cases for different kinds of methods.

@tehsenaus
Copy link

Is this going to be reopened? I think we've made a strong case for abstract properties.

@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus and removed Declined The issue was declined as something which matches the TypeScript vision labels Jan 5, 2016
@RyanCavanaugh
Copy link
Member

Use cases keep piling up indeed. At a hesistant 'yes' but we'll take this to the next design meeting for wider discussion.

@RyanCavanaugh RyanCavanaugh reopened this Jan 5, 2016
@Elephant-Vessel
Copy link

If we get this, the use-case for #4670 could be solved in a clean way as we would be able to redirect the responsibility to fulfill the interface by declaring all parts of the contract (both properties and methods) as abstract and to be implemented by the derived class:

interface A
{
    p;
    m();
}

abstract class B implements A
{
    abstract p;
    abstract m();
}

class C extends B
{
    get p() {return null};
    set p(v) {};
    m() {}
}

Some extra typing compared to the original request, but that could at the same time be seen as increased clarity of intention.

Otherwise I don't know how to cleanly achieve derived classes that have a common abstract class with common behavior and at the same time enforce that the derived classes have certain properties without having unity types everywhere (which feels more like a hack for this scenario anyway).

As I see it, the general concept of the "AbstractClass" is basically a subclass of an "Interface" with the addition of the possibility of having some members implemented. So it would make total sense if "abstract class" indeed is a superset of an interface, which it can not be unless the abstract class can do all the things an interface can do. Just like you can do in C#

interface A
{
    string P { get; set; }
    void M();
}

abstract class B : A
{
    public abstract string P { get; set; }
    public abstract void M();
}

class C : B
{
    public override string P { get; set; }
    public override void M()
    {
        throw new NotImplementedException();
    }
}
@RyanCavanaugh RyanCavanaugh added Committed The team has roadmapped this issue and removed In Discussion Not yet reached consensus labels Feb 1, 2016
@RyanCavanaugh
Copy link
Member

@mhegazy hand this one off?

@mhegazy mhegazy added this to the TypeScript 2.0 milestone Feb 1, 2016
@mhegazy mhegazy assigned sandersn and unassigned mhegazy Feb 1, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Feb 1, 2016

PRs are welcomed also.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 1, 2016

Note, the conclusion form the design discussion was abstract readonly p: number was the desired order of modifiers.

@sandersn
Copy link
Member

#7184 allows for abstract properties (and accessors). There's not much to the change because the machinery is the same as for abstract methods.

@tehsenaus
Copy link

Awesome 👍

Which version is this landing in?

@sandersn
Copy link
Member

sandersn commented Mar 4, 2016

It should already be in typescript@next. It will ship in 2.0.

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Mar 4, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript