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

Allow visibility on constructors #2341

Closed
bpasero opened this issue Mar 13, 2015 · 42 comments
Closed

Allow visibility on constructors #2341

bpasero opened this issue Mar 13, 2015 · 42 comments
Labels
Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript

Comments

@bpasero
Copy link
Member

bpasero commented Mar 13, 2015

I think it is a pretty common pattern to have a static factory method to create a class and the constructor of this class being otherwise private so that you cannot instantiate the class unless you use the factory method.

@qc00
Copy link

qc00 commented Mar 13, 2015

Since all "classes" are actually just functions and since there's no such thing as uncallable function, anywhere the class is visible you can also use the new operator on it.

You can however make the class non-public (e.g. in a module) and export an interface for the class. This abuses the fact that interfaces can extend classes but they're not directly instantiable nor extendable.

@bpasero
Copy link
Member Author

bpasero commented Mar 13, 2015

Well the same applies to private functions in classes, right? I don't see why we could not get a compile error for accessing a private constructor.

@wgebczyk
Copy link

@billccn I don't like to kill requests by "JS allows then you will not be able to hide".
On thing is protect completely in TS & generated JS, other thing is to protect that to the point of generating JS. As you have explained the full protection is not possible, but having different visibility checked by compiler, should be possible.
If you don't like visibility modifiers, then use default public everywhere, there are other that find that concept useful.

@qc00
Copy link

qc00 commented Mar 14, 2015

Yeah, I guess if the private fields are implemented as a compiler check only, then it can probably be extended to constructors. However, the interface-based workaround already works though.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Mar 16, 2015
@dsherret
Copy link
Contributor

👍 There are times when I want to force the programmer to use the factory methods for code readability. The interface-based workaround creates a lot of noise in the code.

I think the compiler check only is the way to go.

@RyanCavanaugh
Copy link
Member

Accepted, accepting PRs

@AbubakerB
Copy link
Contributor

To clarify, could a class with a private constructor be extendable?

i.e. Would this throw an error?

class A {
    private constructor() {
    }
}

class B extends A { // Should there be an error at A saying: "Cannot extend private class 'A'"?
}

if so, would we then allow this:

class A {
    protected constructor(a?: any)
    private constructor() {

    }
}

class B extends A { // No error since 'A' has a non-private constructor
}

@wgebczyk
Copy link

wgebczyk commented Aug 5, 2015

From non-JS developer experience that's expected behaviour.

@RyanCavanaugh
Copy link
Member

In the first example, B should be an error because its implicit super call is illegal. So a class with a private constructor is effectively sealed/final.

In the second example, A's declaration should be an error because all overloads of a constructor, and its implementation, must have the same visibility (same rule we have for methods).

@benliddicott
Copy link

See also #471. Is it really required to allow constructors to be private or will protected do?

@dsherret
Copy link
Contributor

dsherret commented Sep 8, 2015

@benliddicott it's sometimes useful to force a singleton or to force the programmer to use one of the static methods to create an object, because sometimes it can be more readable.

See here.

@benliddicott
Copy link

@dsherret protected meets all those needs.

But you cannot be sure that a downstream user will never have a legitimate need to inherit from your class. The only effect of private is to prevent your downstream users meeting a need you didn't anticipate.

@qc00
Copy link

qc00 commented Sep 9, 2015

@benliddicott Sometimes the only thing you want is for a class to be unextendable. I refer you to Effective Java Item 15 Minimize Mutability, especially:

"2. Ensure that the class can’t be extended. This prevents careless or malicious subclasses from compromising the immutable behavior of the class by behaving as if the object’s state has changed. Preventing subclassing is generally accomplished by making the class final, but there is an alternative that we’ll discuss later.

Currently there's no final/sealed support in TypeScript, so a private constructor is the only way to achieve an immutable class from the type system's perspective. (Though, I do recommend people also freeze the object in the constructor.)

@benliddicott
Copy link

@billccn , that author's opinion is interesting. So is the idea that the library writer's opinion should override that of the library user. My own experience has been that library writers don't know when to use private, and over-use it, causing headaches for users, simply because they believe they know how their library will be used, when in fact they do not.

But rather than a static language like Java, a more apt comparison would be Perl, another dynamic language: http://www.perlmonks.org/?node_id=437623

One of the reasons why perl doesn't have access modifiers such as public, private, and protected, is because it's recognised that they often get in the way of getting the job done: what was envisioned by the original designer has nothing to do with what you want to do with it. In the same way, design for flexibility - although you may not see yourself needing it now, the next person to come along may see it being incredibly handy to solve that new problem, and will bless your genius for developing that flexibility ;-)

and:

Perl doesn't have an infatuation with enforced privacy. It would prefer that you stayed out of its living room because you weren't invited, not because it has a shotgun

http://www.perlmonks.org/?node_id=1096925

In fact JavaScript is the same, and - yes - TypeScript is the same, in almost every respect. In typescript you can access private members just fine - using the aptly named escape-hatch: obj["propertyName"].

If as a library writer you suspect it is unwise to call a method or inherit from an object, then tell the user it is unwise. But don't prevent them - they may know better than you after all.

@bpasero
Copy link
Member Author

bpasero commented Sep 9, 2015

I don't understand the discussion about "protected being sufficient". If TS has the concept of visibilty and I can apply this concept to constructors, the answer is "it is not sufficient".

@dsherret
Copy link
Contributor

dsherret commented Sep 9, 2015

If access modifiers are allowed on the constructor then I think it should have consistent behaviour with other modifiers and allow private.

Private members in general are for sure useful. They allow you to organize and refactor the implementation details of a class without worrying about causing side effects outside the class.

With private constructors I might want to force the developers on my team to program in a certain way. For example, I might want to force them to use the static method here because it's more readable and force them not to extend this class:

class Currency {
    private constructor(private value: number, private type: CurrencyType) {
    }

    static fromNumberAndType(value: number, type: CurrencyType) {
        return new Currency(value, type);
    }

    static fromString(str: string) {
        const value = ...,
              type  = ...;

        return new Currency(value, type);
    }

    // ... omitted ...
}

// error:
const badCurrency = new Currency(5.66, CurrencyType.USD);
// ok:
const goodCurrency1 = Currency.fromNumberAndType(5.66, CurrencyType.USD);
const goodCurrency2 = Currency.fromString("5.66 USD");

@benliddicott
Copy link

With private constructors I might want to force the developers on my team to program in a certain way.

That's a management issue not a language design issue.

@wgebczyk
Copy link

@benliddicott The similar you can say about type descriptors on variables :) If you don't like the feature just use plain JS. OR
Use TS and create some lint-like rules that prohibit usage of private on constructor.To paraphrase your last comment: "That's tooling issue not language design issue".

@dsherret
Copy link
Contributor

@benliddicott if something is not possible to do then I won't have to send it back when it's wrong after doing a code review. That saves time.

Telling the compiler exactly how the code should be used correctly is an asset that gives the proper feedback to the developer using it as they are programming.

@jbondc
Copy link
Contributor

jbondc commented Sep 20, 2015

@dsherret No it's an arbitrary constraint that empowers Architecture astronauts 👎

@jbondc
Copy link
Contributor

jbondc commented Sep 20, 2015

@dsherret Not enough to document instead of enforcing yet another constraint?

It complicates multiple inheritance significantly, see #4805 (which to me seems like an afterthought right now). I've already expressed some of my thoughts in #3578 so won't bother to do it again. Came out a bit strong with astronaut, don't mean to offend anyone.

@dsherret
Copy link
Contributor

@jbondc classes with private constructors wouldn't be able to be inherited. They're essentially sealed and so inheritance, let alone multiple inheritance, shouldn't work with this.

It's much nicer to have self-documenting code than to write it externally or in a comment. That's one of the reasons I like all of TypeScript's restrictions and essentially what we're asking for in this feature—just another way to document the code by using the language itself.

@jbondc
Copy link
Contributor

jbondc commented Sep 20, 2015

Well here is another way to write your example:

module Currency {
    export enum Type {
        CAD = 1.3,
        USD = 1,
    }

    class PrivateCurrency {
        constructor(private value: number, private type: Type) { }
    }

    export function fromNumberAndType(value: number, type: Type) {
        return new PrivateCurrency(value, type);
    }

    export function fromString(str: string) {
        let value = 10;
        let type = Type.CAD;
        return new PrivateCurrency (value, type);
    }
}

Less OO-ish but you actually have a 'genuinely' real private class.

@wgebczyk
Copy link

@jbondc That's great you have found your way for private classes! Let others use another approach (exportable class with private constructor). Now you can observe that there can be features that satisfies needs of group A and do not disrupt group B work. :)

@pleerock
Copy link

+1

1 similar comment
@tobiastimm
Copy link

+1

@AbubakerB
Copy link
Contributor

Is this still on the radar? The PR is hella stale!

@EToreo
Copy link

EToreo commented Dec 18, 2015

+1

1 similar comment
@nbransby
Copy link

+1

@jonaskello
Copy link

+1

@z-vr
Copy link

z-vr commented Jan 24, 2016

👍 can be useful for factory design pattern

@myitcv
Copy link

myitcv commented Feb 2, 2016

Apologies if this is a repeat or just late to the party, but we are using the following pattern to achieve private constructors:

interface ExampleBuilder {
    Instance(): Example;
}

export interface Example {
    Val(): number;
}

export let Example: ExampleBuilder = class ExampleImpl {
    constructor(v: number) {
    }

    Val(): number {
        return 42;
    }

    static Instance(): Example {
        return new ExampleImpl(2);
    }
};

let x = Example.Instance(); // OK: x has type Example
let y = new Example(5);     // ERROR: Cannot use 'new' with an expression whose type lacks a call or construct signature.

Note this can be tidied up even more using the recently merged readonly modifier.

@dsherret
Copy link
Contributor

dsherret commented Feb 2, 2016

@myitcv that's pretty cool to enforce it for now... probably the best way mentioned in my opinion. It's still too verbose though and takes a few seconds to understand what's going on, which is why an access modifier would still be very nice on constructors.

Personally, I'm just marking all my future private constructors with // todo: make private once supported comments and not calling the constructors. It will be nice once this feature comes in order to get some enforcement and better documentation with an access modifier.

@myitcv
Copy link

myitcv commented Feb 2, 2016

@dsherret

It's still too verbose though and takes a few seconds to understand what's going on

Agreed. We don't suffer too much from that cognitive load because these classes are code generated. So the interface to the programmer is actually nice and simple.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 17, 2016

fixed by #6885

thanks @AbubakerB!

@mhegazy mhegazy closed this as completed Feb 17, 2016
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Feb 17, 2016
@mhegazy mhegazy modified the milestones: TypeScript 2.0, Community Feb 17, 2016
@agaace
Copy link

agaace commented Jun 21, 2016

Hi, is this feature going to be released in some future version of typescript?
As of now, trying to declare a private or protected constructor gives me this error in typescript 1.8.10:
error TS1089: 'private' modifier cannot appear on a constructor declaration.

Ahh, nevermind. Just found out the road map that states this feature will be included in typescript 2.0.

@kitsonk
Copy link
Contributor

kitsonk commented Jun 22, 2016

Ahh, nevermind. Just found out the road map that states this feature will be included in typescript 2.0.

Also the milestone being set to TypeScript 2.0 and the label Fixed would indicate that it is included. Anything with a label of Fixed generally is included in master and available via npm install typescript@next.

@stephenmartindale
Copy link

I am using TypeScript 2.0.2 RC and still get TS1089 when I try to make a private constructor. Am I doing it wrong or is this simply not fixed?

@mhegazy
Copy link
Contributor

mhegazy commented Sep 22, 2016

I am using TypeScript 2.0.2 RC and still get TS1089 when I try to make a private constructor. Am I doing it wrong or is this simply not fixed?

it is working for me. make sure your command alias is pointing to the right version, and that your editor is updated to use the latest TS version.

@stephenmartindale
Copy link

I found the problem. It was the fault of gulp-typescript which was using the wrong version of tsc despite my specifications in package.json and what was resolved on the PATH.

For anyone else having this problem, my solution was to edit my gulpfile.js and...

  1. require TypeScript before gulp-typescript as follows:
// Tool-Chain: Scripts
var tsc = require("typescript");
var typescript = require('gulp-typescript');
  1. Provide the compiler as an override when defining my build task:
// Task(s): Build TypeScript Outputs
var tsconfig = typescript.createProject("path to tsconfig", { typescript: tsc });
gulp.task('build:scripts', function () {
    let ts = tsconfig.src()
                     .pipe(sourcemaps.init())
                     .pipe(typescript(tsconfig));

    return ts.js.pipe(sourcemaps.write(".")).pipe(gulp.dest(path.join(outputs.root, outputs.scripts)))
});

@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests