Review Request 110414: Implemented Save Action in Khipu

[Aleix Pol Gonzalez]

> On May 13, 2013, 2:18 p.m., Aleix Pol Gonzalez wrote:
> > src/mainwindow.h, line 99
> > <http://git.reviewboard.kde.org/r/110414/diff/1/?file=143494#file143494line99>
> >
> >     You don't initialize members here. = 0 goes to the constructor.
> >     
> >     Also do we really ned these attributes?
> 
> Punit Mehta wrote:
>     Yes.. we need that because I have implemented it in such a way that we need these variables to keep track of the  number spaces added and saved.!

Yes, I'm saying that you initialize class attributes in the constructor, not in the declaration.


> On May 13, 2013, 2:18 p.m., Aleix Pol Gonzalez wrote:
> > src/mainwindow.cpp, line 374
> > <http://git.reviewboard.kde.org/r/110414/diff/1/?file=143495#file143495line374>
> >
> >     You should _never_ use exit(0).
> >     
> >     I'm unsure what you're trying to do anyway.
> 
> Punit Mehta wrote:
>     I think I have used this to ensure that the application does not crash beacause of the null PlotItem . No problem , I will remove this also .!

exit(0) is equivalent to a crash.


> On May 13, 2013, 2:18 p.m., Aleix Pol Gonzalez wrote:
> > src/mainwindow.cpp, line 422
> > <http://git.reviewboard.kde.org/r/110414/diff/1/?file=143495#file143495line422>
> >
> >     We have API for painting the contents of 2D and 3D graphs into an image. Maybe you should use that.
> 
> Punit Mehta wrote:
>     Aleix , which API you are talking about ? Is it Analitza or some class of Qt ? and is there any problem in the current approach ?

See the methods KAlgebra::saveGraph and KAlgebra::save3DGraph in KAlgebra.

It's only that it's better to make sure we only have 1 set of code paths for printing the view state, and I don't you looked at those.


> On May 13, 2013, 2:18 p.m., Aleix Pol Gonzalez wrote:
> > src/plotseditor.cpp, line 712
> > <http://git.reviewboard.kde.org/r/110414/diff/1/?file=143496#file143496line712>
> >
> >     This looks like terrible API. If you want to do this I'd suggest calling it addPlot and calling plotsModel() from within.
> 
> Punit Mehta wrote:
>     Aleix , I dont get this. You are telling to modify addPlot of analitzaplot ?

No, I'm suggesting to add it to DataStore.


- Aleix


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


On May 13, 2013, 1:55 p.m., Punit Mehta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110414/
> -----------------------------------------------------------
> 
> (Updated May 13, 2013, 1:55 p.m.)
> 
> 
> Review request for KDE Edu, Aleix Pol Gonzalez and Percy Camilo Triveño Aucahuasi.
> 
> 
> Description
> -------
> 
> As we discussed the outline for khipu-specific file , I have implemented this feature. The attached diff contains the feature for save Action in Khipu. I have used qjson for file format.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt f7dc583 
>   src/CMakeLists.txt 0a50c54 
>   src/datastore.h a1276a7 
>   src/datastore.cpp 616e0cc 
>   src/mainwindow.h 1f059f8 
>   src/mainwindow.cpp 9d6e010 
>   src/plotseditor.cpp 106cdf5 
> 
> Diff: http://git.reviewboard.kde.org/r/110414/diff/
> 
> 
> Testing
> -------
> 
> Done..! worked perfect for any situtation I have come across. !
> 
> 
> Thanks,
> 
> Punit Mehta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20130514/cc5acd03/attachment.html>