Community
Participate
Working Groups
When invoking clean up with "Remove unused private member - constructor" enabled on the example class from the clean up preference page, the constructor is not being removed. class Example { private Example() {} public void bar() { int i= 10; } }
That's not a bug. That's a feature. If you remove the private Example(), then you're implicitly creating a public constructor (public Example()) instead. So you've gone from a situation where no-one else can instantiate the class to a situation where everyone can create instances of that class. If this were 'fixed', you'd pretty much be guaranteed to break every Singleton class there is ...
Thanks Alex, interesting... Here: class E1 { private E1(int i) {} } class E2 { public E1 e= new E1(); } The private constructor is flagged as never used locally (and therefore removed by clean up). Which is basically correct: It is never used locally. But why is then the constructor without a parameter not flagged? Moving to core for comments. Fix on the clean up side is to change the example in the preview.
Re #1: Of course you're right, my example was too simple. Considering the following code: class Example { private Example() { } public Example(int i) { } } Clean Up should remove the private constructor, right?
I agree our behavior is suboptimal. We only tolerate a private constructor with no parameter, as known pattern to block instantiation. In fact, it should tolerate private constructor (any params) if sole constructor.
(In reply to comment #3) > class Example { > private Example() { > } > public Example(int i) { > } > } > > Clean Up should remove the private constructor, right? > Clean Up does remove all constructors which are marked as unused by the compiler. The above constructor is not marked, although it think it can be marked safely. I guess core does just never consider private unparameterized constructors.
re: comment 4 I think it should flag unused private constructors as soon as there are non-private constructors as well. Unsure about whether it needs to be the sole constructor. i.e. should it complain for the following case ? class E1 { private E1(int i) {} private E1(long l) {} }
Created attachment 56627 [details] Proposed patch Patch for 3.3 stream, which only blame unused private constructors if non-private one(s) are also present. So it will not complain on last testcase above with 2 private constructors.
(In reply to comment #6) > I think it should flag unused private constructors as soon as there are > non-private constructors as well. Agree. And not flag them if there are no non-private constructors. > Unsure about whether it needs to be the sole > constructor. > i.e. should it complain for the following case ? > > class E1 { > private E1(int i) {} > private E1(long l) {} > } > IMHO it should tolerate them and not complain. Removing (both) constructors would change the semantic, they are therefor not unnecessary. But of course it makes no sense to write unused constructors with parameters, but this would be another kind of warning, wouldn't it?
Likely yes, another warning (separate issue). Need to tune quite a few existing test expectations then. Eric - can you attach a patch with updated affected tests ?
Created attachment 56716 [details] Better patch
Released for 3.3M5
Verified for 3.3 M5 using build I20070206-0010
