On Tuesday 23 March 2010 12:15:30 P Zoltan wrote:
> I've looked more closely to the source code of kde4-port, and observed a
> strange thing: CircuitModel needs an instance of Kdevelop::Core, and
> internally has a pointer to a Circuit plugin object. In my opinion this
> can cause problems for test case writing, because then running tests
> supposes a working copy of ktechlab, and maybe more.
> Is that really necessary? In my opinion a circuit model shouldn't know
> about a kdevplatform plugin. Maybe the code can be reworked in order to
> have a relatively easy to use CircuitModel class.
The KTLCircuitPlugin knows about the electronic components it is able to
handle. The Model doesn't know about this, so it has to get information from
the CircuitPlugin (the filename of the SVG file, in this case). KDevPlatform is
needed, because more components can be provided by new Plugins. KDevPlatform
does all the plugin-loading magic, so I guess we need an instance here, or
provide this functionality by ourselves, which I would like to avoid. However
for testing, the test can create a KDevelop::Core instance without a GUI. This
will create something that is able to handle projects and documents and load
Plugins (which don't require a GUI). To compile KTechLab, you will need
KDevPlatform installed, anyway.
I don't see an easy way to change this, without bringing back the "one class
for each component" problem, which IMHO doesn't scale.
> In very long term I'd like to have a "light weight" version ktechlab,
> with a more simplistic GUI, which would depend only on QT. That could come
> useful for testing the core functionality, and it would be cross-platform.
> This is yet another reason why I'd prefer a layer of abstraction
> independent from KDE.
You would have to re-implement most parts of KDevPlatform in this case. This
would be something like: ProjectManager, DocumentManager, PluginManager. These
are IMHO all things, we don't want to care about, since we want to provide
something to develop and simulate electronic circuits and program MCUs. But
well, of course I'm willing to do all this with Qt-only libraries, for now I
just don't see, how this can be done without rewriting the parts of
KDevPlatform, we use.
> Also there are some files where multiple classes defined in them, for
> example CircuitDocument and CircuitDocumentPrivate, KTL*Factory. This
> shouldn't be a problem by itself, as those are internal classes, but the
> order they appear in the file might be confusing. Maybe make them inner
There is a difference between CircuitDocument(Private) and these Factory
classes. Providing private class implementation for a class withing a Plugin
is not mandatory. In this case, the private classes are used to remove the
implementation from the class (API) definition and implementation. This is
needed for all classes in src/interfaces/ to provide binary compatibility. For
Plugins (which are implementations of these interface classes), this is not
needed. I learned about that after creating private class for CircuitDocument,
so I didn't change it, yet ;) If these classes grow to large, they can be
separated into their own files named like classname_p.(h|cpp).
Concerning the Factory classes, these are mostly generated by cpp macros
provided by KDE and KDevPlatform. These classes are used to dynamically create
instances of the Plugins classes which themselves implement some public
interface. It's not possible to make them inner classes. In the case of the
private class implementations, this would just bring the problem back, that it
solves (resulting in binary incompatibility, if something changes withing the
private class). In the case of the Factory classes, they are used by the
KDevPlatform to create the Views to be displayed withing the Shell or create
some extra entries for the context-menu. I don't know, if this will still
work, when hiding them within some class.
Can you provide a patch (in a new branch) to demonstrate, what you would like
to change? As I said, it should be okay to merge CircuitDocument and
CircuitDocumentPrivate, but for the Factory classes, I think they should stay
as they were. This is also the way these are used throughout KDE code. May be,
I just miss your point here and you are right and we should change some
> Yet another note: the predefined GNU GPL headers are not completed
> everywhere in the source files; it would look better if that would be done.
I mentioned that, too :S Sometimes I just forgot about it. There should also
be 2 different forms of these headers, because I used vim (and some macros
doing this stuff for me) before KDevelop became usable, again.. I wanted to
clean these things up, one day. For now, this has low priority for me...
> What is your opinion?
Well, thanks for having a look into the code. May be, it's time to start with
the review-based work-flow. Meaning, from now on I will only commit patches
into the kde4-port branch, after they have been discussed on the ML. This
should prevent such things like the Private class for an implementation,
because I would have needed to explain why I did it, before it hits the kde4-