Hello, this article is a sequel to the part 1 of the project I have undertaken with LibreOffice. If you don’t know what I’m talking about, you can refer to the following URLs to read more about “it”:
The second phase was code refactoring phase, in which I had to separate out the logic of the core gallery classes from reading and writing files and directories.
The phase lasted for 2 months, which was earlier planned to be finished in 1 month. What a stretch! But as you will read further, you may realize the reason behind the stretch.
Some basic what, why and how questions arise — you’re lucky that I’m here to clarify.
What is Refactoring?
In computer programming and software design, code refactoring is the process of restructuring existing computer code — changing the factoring — without changing its external behavior. Refactoring is intended to improve the design, structure, and/or implementation of the software (its non-functional attributes), while preserving its functionality. [Source: Wikipedia]
I hope this answers the first question. Basically, improving the structure by introducing classes and putting things to where they actually belong. This makes the code easier to maintain, reduce time of bug fixing and in this case, will help me to implement my upcoming interface.
The gallery code needs to be refactored in order to make it easier for me to implement XML+ZIP Engine class. It will become clearer to you in some time.
Objectives of Refactoring?
The objective is to make separate classes that deal with reading and writing of files. This should leave the core classes carrying logic in a single chunk of code. This is the answer to the “how”.
Let’s take a look at the old gallery structure of LibreOffice. It’s a rough UML diagram prepared by me which mentions few important classes and their members. Here’s the link to the chart if this one isn’t clear to you.
Notice from the figure that class GalleryTheme() has SotStorage, which clearly belongs to a different class. It also has GetThmURL(), GetSdgURL(), etc. which are dealing with URL. Similarly in GalleryThemeEntry — it can be observed that those methods and member variables do not belong to that class. So I created a new class called GalleryBinaryEngine that would be instantiated by GalleryTheme and a class called GalleryBinaryEngineEntry that would be instantiated by GalleryThemeEntry. These new classes will have the read/write file stuff in it.
Let’s look at an example of how a function is refactored. Click here to see the code in the picture I’m referring to.
In the code above, GalleryTheme::CreateThemeEntry() is a function that adds an entry of the theme to the theme list. Now, line 652 creates an input stream… hmm.. seems like a good candidate for refactoring. This code should be put inside GalleryBinaryEngine as the whole function is about that. Besides, it’s a static member so that makes my job easier!
Note that in this example, I moved the whole function out of the GalleryTheme class. But mostly that’s not the case. I have moved “functionalities” instead of the whole functions in this phase. These functionalities deal with R/W stuff so they needed to be moved out of it. So that’s an interesting part..
The further refactoring work was done in similar fashion. After every cycle of patch I was sending, I was running my tests (CppUnit_svx_gallery_test) to make sure I did not break the code and all is well. I feel glad that I wrote tests beforehand. :)
After the refactoring was almost done (or that’s what I thought) the structure looked like below (Click here for a detailed picture)
But as I was finalizing the refactoring, I noticed that there is a member called m_aObject list inside GalleryTheme that GalleryBinaryEngine was also requiring frequently. So I had to introduce a new class called GalleryObjectCollection that would contain this member, primarily owned by GalleryTheme. The reference would be passed over to GallerybinaryEngine at construction. Then there was another thing that I noticed; I realized that there was aThmURL,etc. needed by GalleryBinaryEngine, but it was owned by GalleryBinaryEngineEntry. Hmm.. so how do I make it in such a way that the URLs are shared all the way from GalleryBinaryEngineEntry to GalleryBinaryEngine. It was my genius mentor, Tomaž’s idea to create a new class which would store these URLs and implement setters and getters in it. So GalleryStorageLocations was formed and it was primarily owned by GalleryThemeEntry, the reference of which was passed on to GalleryBinaryEngineEntry and GalleryBinaryEngine at construction. After updating the structure and doing most of the refactoring work, the final gallery structure looked like this (Click here for a detailed image)
Yeesh! With that, the final gallery structure is formed. This is looks nice because reading and writing bits are almost completely separated out from the logic part. Of course, there are some final changes left to be done which I will be doing alongside the next phase. According to my mentor, I have left a lot of things in refactoring. I feel appalled cause the phase already took 2 months.
- Around 40 functions were introduced by me during this phase, contributing ~1000 lines of code.
- The code is much simpler now and easily maintainable. This will help the developers seeking to work in the gallery module.
- Bug fixing will now be much easier, saving up time.
- I will now be able to implement XML Engine easily, because the functions of it would be alternate to Binary Engine.
- …and more to come
This phase really took longer than expected. As expected, the feedback for this month wasn’t great. But it’s okay, I will work harder this month and won’t waste a single day. I believe that I can accomplish the project before the summer of code ends. I have learnt a lot in these two months but I have yet much more to learn.
Now that the code is refactored, I will implement XML Engine which will deal with XML files. I will learn how to read and write from XML files and how to create a ZIP file, insert files into it, read from the files, etc. After this is done, I can finally build the interface, GalleryInterface whose functions will be implemented by XML Engine and BinaryEngine. If this is done, 90% of my project will be completed.
It’s a race against time.
I hope you enjoyed reading the article. I enjoyed writing them too! I’m planning to contribute to LibreOffice even after GSoC ends because I love the software and the whole open source community!