This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
I noted that (perhaps a new feature) refactoring includes changing method arguments. I started with: class TlTrackSegment { ... virtual bool getMediaOffset(const TlClipDesc&, const MediaPlayReq& req, MediaSampleDesc& sDesc, bool& success); and I wanted to change it to this: virtual bool getMediaOffset(uint level, const TlClipDesc&, const MediaPlayReq& req, MediaSampleDesc& sDesc, bool& success); The problems resulted as follows: - The dialog insists on a default value for the added argument, which is not required for c++. This should be an optional part of the refactoring. See 1st attachment. - in derived classes it puts the default argument in /* */. This would be bad style. Changing the signature (optional vs not) in derived classes means that when using the derived class pointer, the optional argument is no longer optional. This should be an optional part of the refactoring. see second attachment. - when I move the argument to 1st position it still generates an optional argument in the declaration. This is invalid c++. See third attachment. - in the base class implementation, the default argument appears in /* */. Also very bad style, because that would suggest that the implementation can always expect the optional argument. This should be an optional part of the refactoring. See fourth attachment.
Created attachment 106683 [details] function paramter dialog
Created attachment 106686 [details] derived class declaration
Created attachment 106687 [details] base class declaration
Created attachment 106688 [details] base class implementation
Also, the refactor affected the usages of the refactored method. But it did so by adding only a comma after the last (existing argument). For example, one usage was refactored from: if (! children[i]->getMediaOffset(req, localSampleDesc, thisSuccess)) to if (! children[i]->getMediaOffset(req, localSampleDesc, thisSuccess,))
Product Version: NetBeans IDE Dev (Build 201102170501) Java: 1.6.0_23; Java HotSpot(TM) Client VM 19.0-b09 System: Linux version 2.6.35.10-74.fc14.x86_64 running on i386; UTF-8; en_US (nb) Userdir: /home/toddb/.netbeans/dev
Created attachment 106771 [details] use case where I can not reproduce the issue Todd, Based on your screenshots I've created test case project, but I can't reproduce your issue. Could you, please, modify it somehow, so I can reproduce the problem. Thanks, Vladimir.
(In reply to comment #5) > Also, the refactor affected the usages of the refactored method. But it did so > by adding only a comma after the last (existing argument). For example, one > usage was refactored from: > > if (! children[i]->getMediaOffset(req, localSampleDesc, thisSuccess)) > > to > > if (! children[i]->getMediaOffset(req, localSampleDesc, thisSuccess,)) Just in case, what you've specified finally as default value? I've realized that if I put space as default value => I will have issue like yours. Btw, default value is the value which is inserted automatically in all function calls. And I agree that it's useless to change signatures with /* = def */ style.
(In reply to comment #0) > I noted that (perhaps a new feature) refactoring includes changing method > arguments. I started with: > class TlTrackSegment { > ... > virtual bool getMediaOffset(const TlClipDesc&, > const MediaPlayReq& req, > MediaSampleDesc& sDesc, > bool& success); > > and I wanted to change it to this: > virtual bool getMediaOffset(uint level, > const TlClipDesc&, > const MediaPlayReq& req, > MediaSampleDesc& sDesc, > bool& success); > > The problems resulted as follows: > - The dialog insists on a default value for the added argument, which is not > required for c++. This should be an optional part of the refactoring. See 1st > attachment. It is required, because specified value should be inserted in all existing function calls to prevent breaking of compilation. > - in derived classes it puts the default argument in /* */. This would be bad > style. Changing the signature (optional vs not) in derived classes means that > when using the derived class pointer, the optional argument is no longer > optional. This should be an optional part of the refactoring. see second > attachment. I will remove it unless check box "... only in Declarations" is checked. > - when I move the argument to 1st position it still generates an optional > argument in the declaration. This is invalid c++. See third attachment. As above. Default means what to use in updated "calls", not in updated signatures. So it's needed, right? > - in the base class implementation, the default argument appears in /* */. > Also very bad style, because that would suggest that the implementation can > always expect the optional argument. This should be an optional part of the > refactoring. See fourth attachment. Will fix this part
Integrated into 'main-golden', will be available in build *201103140400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main/rev/79b8b6be8f0a User: Vladimir Voskresensky <vv159170@netbeans.org> Log: fixing #196268 - refactoring method arguments produces bad code -- use constants instead of numbers