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 overwritten in sub class are undefined within parent constructor #15247

Closed
PMXScott opened this issue Apr 18, 2017 · 10 comments
Labels
Duplicate An existing issue was already created

Comments

@PMXScott
Copy link

TypeScript Version: 2.2.1 / nightly (2.2.0-dev.201xxxxx)

Code

abstract class A {
	public abstract names:string;
	public constructor() {
		console.log(this.names);
	}
	public speak() {
		console.log(this.names);
	}
}

class B extends A {
	public names:string= "scott";
}

new B().speak();

Expected behavior:

Output to be

scott
scott

Actual behavior:

undefined
scott
@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Apr 18, 2017
@RyanCavanaugh
Copy link
Member

See #1617

@PMXScott
Copy link
Author

@RyanCavanaugh thanks :) I feel due to the similar nature of the bug report you made me aware of (which I also commented on) that I should justify this bug report. This one is for an abstract property (the other bug report is not). Since an abstract property must be extended in a sub class. The current implementation means it would never have a property accessible within the constructor of the base class and clearly shows it will be overwritten (and enforced) so it makes no sense to declare it in the base class in the first place.

In my opinion, not having access to such an extended property within the parent constructor seems nonsensical especially given the nature of an abstract. What would provide a clearer indication to the access (scope) of such a property would be to add the property to an interface instead of marking as abstract allowing it to be completely removed from the base class making it less likely a programmer will accidentally try to access it within the base class constructor. But, if they did try to access it they would then fall prey to a bug of similar nature to #1617.

It's crazy how many gotchas are appearing because of the way this sub classing around properties are implemented. Yet the reasoning behind why it was implemented this way appear due to technical reasons and not for reasons of what the community was expecting.

I feel abstract properties with the way they are currently implemented is very easy for someone using them to be tripped up.

@RyanCavanaugh
Copy link
Member

There is not any other good option available; every other programming language with subclassing allows but behaves poorly in the presence of virtual property/method access in the constructor.

@PMXScott
Copy link
Author

Hello @RyanCavanaugh What do you mean when you say "behaves poorly"? I am assuming (though correct me if I'm wrong) that you are referring to performance of code execution. Until then, for the sake of this comment I will go along the basis that you are implying such performance.

If that's so, wouldn't the impact of applying workarounds be of more detriment to the overall performance? In addition to the wasted developer time in debugging such unexpected behaviour?

@RyanCavanaugh
Copy link
Member

It's not a performance issue, it's a correctness issue. It's all spelled out at #1617 (comment)

@PMXScott
Copy link
Author

Isn't what you pointed out a current implementation? Not necessarily how it "should" be. It's also not how more common / accepted / traditional styles of OO implementations are done. Of course, I am somewhat generalising.

@RyanCavanaugh
Copy link
Member

There are two possible things, and we picked the one with the least-bad side effects.

Every OO language that I'm aware of follows the principle that the base class initializes before the derived class, and that virtual behavior may be inconsistent when done from the base class constructor. We're not unusual in this regard at all.

@ghost
Copy link

ghost commented Apr 19, 2017

That's interesting you say that. As it means you won't be aware of how well or how badly such an implementation as suggested in this bug report would work in real world scenarios.

Here are two different language examples where they happen to do things in such as way as I've suggested.

Python 3

class A:
    name = ''
    def __init__(self):
        print(self.name)
        
class B(A):
    name = 'scott'
    
B() # prints "scott"

PHP 7

class A {
    public $name;
    public function __construct() {
        print $this->name;
    }
}

class B extends A {
    public $name = "scott";
}

new B(); // prints "scott"

I'm not aware of any such issues as raised in these bug reports, or any required workarounds due to either of those languages doing it the way they have and as I've suggested here.

Might be something for the TypeScript team to at least reconsider in the future. Avoiding the "workarounds" that people have already mentioned and improve the purpose of the abstract keyword being added to properties. Why declare abstract properties in the parent class and be able to access them within the parent construct when you know they will never be accessible, at least throw an error if nothing else which further suggests the implementation wasn't fully envisaged in its entirety. Maybe that should be, if it's not already logged, an enhancement request?

P.S. I've noticed you're quite a contributor @RyanCavanaugh to TypeScript. You and the TypeScript team are doing great work! Having used TypeScript for a short time now I believe it's more like what JavaScript should have been from the beginning. Given time, hopefully I will be able to contribute code back to the TypeScript repository 👍

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Apr 19, 2017

Why declare abstract properties in the parent class and be able to access them within the parent construct when you know they will never be accessible, at least throw an error if nothing else which further suggests the implementation wasn't fully envisaged in its entirety.

There's nothing that prevents people from writing this:

abstract class Base {
    abstract name: string;
    abstract dangerousVirtualInit();

    constructor() {
        this.dangerousVirtualInit();
        console.log(this.name);
    }
}

class Derived extends Base {
    name: string;
    dangerousVirtualInit() {
        this.name = "Derived";
    }
}

new Derived(); // Prints 'Derived'

As it means you won't be aware of how well or how badly such an implementation as suggested in this bug report would work in real world scenarios.

In programming languages you're sort of stuck doing thought experiments. We know that a different initialization order would cause this problem:

class Base {
    x = 'foo';
    getX() {
        return this.x;
    }
}
class Derived extends Base {
    m = this.getX(); // Would be 'undefined', not 'x'
    constructor() {
        super();
    }
}

which is really just unacceptable -- Base should never be able to observe itself in a state where its properties have been uninitialized. The only problem we have is people attempting to observe virtual behavior from a base class constructor, which, again, is something that OOP languages almost universally discourage.

Again, #1617 (comment) spells out what the available solutions are. TS/JS is subject to constraints that PHP and Python aren't. People keep clamoring for solutions which are not available, in opposition to the technical evidence, and it's very frustrating. Like, we didn't choose a bad behavior on purpose - we chose the best behavior among the options which are possible.

@ghost
Copy link

ghost commented Apr 19, 2017

In programming languages you're sort of stuck doing thought experiments.

Can't disagree with you there.

Base should never be able to observe itself in a state where its properties have been uninitialized.

Brains now scrambled :p

People keep clamoring for solutions which are not available, in opposition to the technical evidence, and it's very frustrating.

Programmers ay ;) This is TypeScript, can you not implement it how you like? Just winding you up :p

I've just read this (sorry, should have read this before continuing this discussion): https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals

Align with current and future ECMAScript proposals.

Guess you have no choice ;) I was somewhat assuming you had a bit more flexibility in applying restrictions using the compiler and the typescript way to help guide people down a friendlier path but appears your hands are tied.

I would suggest adding in some more compile time error checks though around accidental misuse/misunderstanding of typescripts abstract implementation. If nothing else is gained from this discussion.

Thanks for your time @RyanCavanaugh in discussing this. Didn't intend to provoke you especially since you're drastically more experienced on this subject than myself. No doubt will be bumping into you again around here some time :)

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

2 participants