Review Request 124300: Change signature
Maciej Poleski
d82ks8djf82msd83hf8sc02lqb5gh5 at gmail.com
Sun Jul 26 12:21:34 UTC 2015
> On Lip 26, 2015, 11:19 rano, Milian Wolff wrote:
> > refactoring/changesignaturedialog.cpp, lines 201-202
> > <https://git.reviewboard.kde.org/r/124300/diff/2/?file=385182#file385182line201>
> >
> > I suggest you introduce an `enum Columns` to the model with `Type` and `Name` enumerators and use them here instead of magic numbers.
But Qt API uses `int` to denote column. How to use enumeration instead?
> On Lip 26, 2015, 11:19 rano, Milian Wolff wrote:
> > refactoring/changesignaturedialog.cpp, line 276
> > <https://git.reviewboard.kde.org/r/124300/diff/2/?file=385182#file385182line276>
> >
> > what do you mean by leak here? not memory, I guess?
memory.
I remove reference to element of `m_changePack->m_newParam` vector. This element will not be referenced anymore but uses space. I decided not to remove this element from `m_changePack->m_newParam` because i would need to update all references to new parameters which changed position in this vector. It would mean more code (bugs), more computations and not necessary worthy result. In practice user will introduce at most a few new parameters and at most a few of them will be removed using dialog. It is no more than 100B of memory.
Anyway this memory (including leaked elements) will be freed (as part of vector) after refactoring is done.
- Maciej
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124300/#review82979
-----------------------------------------------------------
On Lip 13, 2015, 11:34 po południu, Maciej Poleski wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124300/
> -----------------------------------------------------------
>
> (Updated Lip 13, 2015, 11:34 po południu)
>
>
> Review request for KDevelop.
>
>
> Repository: kdev-clang
>
>
> Description
> -------
>
> Change signature refactoring
>
> I decided not to implement rename function refactoring. Change signature will provide this functionality.
> We can rename all functions except constructors, destructors, conversion operators, operators.
> Operators must preserve number of parameters (imagine 2+2 and operator+).
>
> This functionality allows to:
> - change return type
> - change function name
> - change parameter types (preserving identity, tracking of this is not perfect - we lose this information)
> - change order of parameters
> - introduce new parameters
> - remove existing parameters
>
> Somewhere here should be screenshoot of this dialog...
>
>
> Diffs
> -----
>
> refactoring/CMakeLists.txt PRE-CREATION
> refactoring/changesignaturedialog.h PRE-CREATION
> refactoring/changesignaturedialog.cpp PRE-CREATION
> refactoring/changesignaturedialog.ui PRE-CREATION
> refactoring/changesignaturerefactoring.h PRE-CREATION
> refactoring/changesignaturerefactoring.cpp PRE-CREATION
> refactoring/changesignaturerefactoringchangepack.h PRE-CREATION
> refactoring/changesignaturerefactoringchangepack.cpp PRE-CREATION
> refactoring/changesignaturerefactoringinfopack.h PRE-CREATION
> refactoring/changesignaturerefactoringinfopack.cpp PRE-CREATION
> refactoring/debug.h PRE-CREATION
> refactoring/refactoring.h PRE-CREATION
> refactoring/refactoring.cpp PRE-CREATION
> refactoring/refactoringmanager.cpp PRE-CREATION
> refactoring/utils.h PRE-CREATION
> refactoring/utils.cpp PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/124300/diff/
>
>
> Testing
> -------
>
> Manual testing of GUI. No transformation tests yet...
>
>
> File Attachments
> ----------------
>
> snapshot
> https://git.reviewboard.kde.org/media/uploaded/files/2015/07/08/e8a8db09-92f9-4fb6-ac4a-3ab2a5051033__snapshot1.png
>
>
> Thanks,
>
> Maciej Poleski
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150726/ed76f3a4/attachment-0001.html>
More information about the KDevelop-devel
mailing list