Review Request 110992: Changed some code in plotsdictionarymodel.cpp and plotsdictionary.h to have better dictionary support

Aleix Pol Gonzalez aleixpol at kde.org
Thu Jun 13 13:08:38 UTC 2013



> On June 13, 2013, 11:44 a.m., Aleix Pol Gonzalez wrote:
> > analitzaplot/plotsdictionarymodel.cpp, line 51
> > <http://git.reviewboard.kde.org/r/110992/diff/2/?file=149817#file149817line51>
> >
> >     I don't think I like to specify the dimension here. We can get the (available) dimensions for a said expression, why do we need to enforce a dictionary dimension when we can decide it on runtime.
> >     
> >     If we want to specify it for the dictionary, I'd suggest to create separate .desktop files that describe the dictionaries, but having the metadata there is not good. Especially if you're suggesting to put a line such as dimension := 2 and then you only check for contains("2")
> 
> Punit Mehta wrote:
>     >>I don't think I like to specify the dimension here. We can get the (available) dimensions for a said expression, why do we need to enforce a dictionary dimension when we can decide it on runtime.
>     
>     -> Yes.. we can decide it on run-time ( As you have also suggested, using  Analitza::PlotsFactory::self()->requestPlot(exp, Analitza::Dim2D).canDraw() ) .. but I think this may slow down the process of plotting in the application because , we need to keep checking about the dimension and request for the plots whenever the user switches in different plot-names and accordingly change the model for the views ( from PlotsView2D to PlotsView3D and vice versa ). 
>      
>     -> Where as , it  is very likely that each plot file will contain the plots of the same dimension.
>     
>     If we don't want to have the dimension for the dictionary still , then I can even change the code :) and can find an alternate way , but as I said , it might slow down the execution :)

We won't know if it slows down until we try it :)


- Aleix


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110992/#review34283
-----------------------------------------------------------


On June 13, 2013, 12:51 p.m., Punit Mehta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110992/
> -----------------------------------------------------------
> 
> (Updated June 13, 2013, 12:51 p.m.)
> 
> 
> Review request for KDE Edu, Aleix Pol Gonzalez and Percy Camilo Triveño Aucahuasi.
> 
> 
> Description
> -------
> 
> Improvement in the code of plotsdictionarymodel class in analitzaplot.
> 
> 1) Initially , the constructor itself creates the dictionary. Now , the constructor just initializes the object and user/programmer can create the dictionary according to the wish and the flow of the program.
> 2) Also , I have added some improvements to support plotting from the dictionary for both the dimensions ( dim2D and dim3D ). For that , the assumption is that the dictionary file ( .plots file ) will contain the dimension in the first line as a comment ( e.g. [// dimension :=2 //] code in the first line in .plot file).
> 3) I have also added some other dictionary files ( for dim2D and dim3D )  so that other application based on Analitza ( in my case Khipu ) can use the dictionary support very well. These dictionary files are just for testing purpose. I am still looking forward in creating a rich collection of dictionary files. 
> Here, the dictionary files will be installed in the directory whose path will be like : [ default-kde-path / libanalitza /plots ].  
> 
> -> Once , the change will be merged in analitzaplot , khipu can also work very well as far as the dictionary support is concerned. :) 
> 
> 
> Diffs
> -----
> 
>   analitzaplot/data/plots/3Ds.plots PRE-CREATION 
>   analitzaplot/data/plots/basic_curves.plots 42d8ec7 
>   analitzaplot/data/plots/conics.plots PRE-CREATION 
>   analitzaplot/data/plots/polar.plots PRE-CREATION 
>   analitzaplot/plotsdictionarymodel.h a308cdc 
>   analitzaplot/plotsdictionarymodel.cpp 3e99167 
> 
> Diff: http://git.reviewboard.kde.org/r/110992/diff/
> 
> 
> Testing
> -------
> 
> Done.. works in khipu .. but I am not sure if other part of analitza uses plotsdictionarymodel. If that is the case , then we need to change the code for that class also . but as fas as I know , there is not any other class which depends on plotsdictionarymodel in analitza.
> 
> 
> Thanks,
> 
> Punit Mehta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20130613/7d0e5773/attachment-0001.html>


More information about the kde-edu mailing list