Mailing lists / lazarus / 2010-01
9 messages · 4 people · –
Reply by email Subscribe to this list Your subscription
Read-only archive. Use j and k to move between messages.
I continue refactoring of PropEdits unit.
I am ready to extract TPropertyEditHook into a separate unit. Although Paul have given me commit access to IDEIntf, he requested that I ask before committing serious changes. So, is it ok to commit this refactoring?
Zitat von Alexander Klenin :
No, it breaks compatibility.
Why do you want to move it to another unit?
Mattias
2010/1/22 Mattias G?rtner :
What compatibility?
I have successfully re-built lazarus after the refactoring -- no files except propedits.pp required any changes.
Why do you want to move it to another unit?
Because propedits.pp is a 6000+ line monster which is very hard to comprehend, fix and improve.
Reducing the size by 1200 lines will alleviate the problem somewhat. Also, many units that currently import propedits are actually only using TPropertyEditHook, so after the extraction those units can import much lighter propedithook unit instead. Note that the patch I have currently prepared does not implement the latter change yet, but it will follow shortly.
Zitat von Alexander Klenin :
Amazing, TPropertyEditHook is used in some IDE units. What exactly do you want to move?
I can't follow you.
a) TPropertyEditHook is not used anywhere then moving will not break compatibility
b) TPropertyEditHook is used in many units then moving will break compatibility
With "compatibility" I mean the designtime packages that use IDEIntf.
What about GlobalDesignHook?
Note that the patch I have currently prepared does not implement the latter change yet, but it will follow shortly.
ok. Hopefully it will clarify some things. :)
Mattias
2010/1/22 Mattias G?rtner :
At the current stage, I simply re-export TPropertyEditHook from propedits unit.
Theoretically, after updating all dependent units
this re-export might be removed, but there is no pressure -- it is only a single line and can be postponed indefinitely.
My change _allow_ units to depend only on propedithook, but does not require it.
What about GlobalDesignHook?
That is indeed a problem, and I have a plan for it ;-) However, at the first stage I left in intact.
> Note that the patch I have currently prepared does not > implement the latter change yet, but it will follow shortly. ok. Hopefully it will clarify some things. :)
I hope so too ;-)
Refactoring should not be the final goal. Refactoring usually used when something fit bad into the existing design. As Mattias I don't see the big need to extract all the propedit internals to other units - it will complicate the use of IDEIntf for other developers. Although some non-related to property editors code can be moved away.
Best regards,
Paul Ishenin.
That is one possible reason.
The other reason (which was the actual reason I undertook the refactoring) is that, at least for me, there is a certain threshold of ugliness after which I am just
unable to unravel the current design and understand it enough to make a needed change, even if the design itself can accommodate it. This property is often called 'unmaintainability'. Of course, the threshold is different for different people, but I consider 6000 LOC in a singe unit to be far beyond any reasonable level.
As Mattias I don't see the big need to extract all the propedit internals to other units - it will complicate the use of IDEIntf for other developers.
Hm. Are you sure of that? I am, in a sense, "other developer", because I tried to use IDEIntf to implement custom editor for TAChart, and I specifically wanted to make PropEdits easier to understand and use.
So, why do you think proper layer separation will make the PropEdits harder to use?
Although some non-related to property editors code can be moved away.
Yes, I started this with PropEditUtils extraction, but there is more to do in this direction.
General speaking:
Readability alone is a bad reason to break compatibility. Readability can be improved by comments, class-trees, examples, tutorials and include files.
[...]
Mattias
Readability is just one factor contributing to maintainability. PropEdits code is actually rather good in that aspect. But to understand the design proper modularity is very important too. PropEdits is a tangled web of interdependent classes, and it is hard (to me, almost impossible) to have a clear mental picture of all these dependencies.
Such a picture is required to understand and fix the bugs and to develop new features.
I wanted to improve the modularity by separating code into a layered units and breaking circular dependencies.
Anyway, this is not a great problem, so I give up. I will instead submit bugs to Mantis in the hope that some person smarter than me
will be able to fix them ;-)
P.S. What is the "class-tree"?