hi, 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 > classes? 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 detail. > 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- port branch. bye then julian ------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev _______________________________________________ Ktechlab-devel mailing list Ktechlab-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ktechlab-devel
