Add dptr to KReportData and SortedField
ClosedPublic

Authored by piggz on Dec 20 2016, 11:40 AM.

Details

Summary

Extended SortedField with methods and a dptr

Test Plan

Ran kreportexample

Diff Detail

Repository
R14 KReport
Branch
dptr-final
Lint
No Linters Available
Unit
No Unit Test Coverage
piggz updated this revision to Diff 9211.Dec 20 2016, 11:40 AM
piggz retitled this revision from to Add dptr to KReportData and SortedField.
piggz updated this object.
piggz edited the test plan for this revision. (Show Details)
piggz added a reviewer: staniek.
Restricted Application added a project: KReport. · View Herald TranscriptDec 20 2016, 11:40 AM
piggz updated this revision to Diff 9231.Dec 20 2016, 6:06 PM
  • Added dptr to KReportDesignReadingStatus
staniek requested changes to this revision.Dec 20 2016, 9:27 PM
staniek edited edge metadata.

Thanks.

src/common/KReportDesign.cpp
33

+Q_DECL_HIDDEN

36

How about moving these docs to KReportDesign.h for the users?

43

-> errorLineNumber

46

-> errorColumnNumber

49

Minor, for the future: it's enough to use new Private without ()

src/common/KReportDesign.h
47

->

Equivalent of errorLineNumber() >= 0 && errorColumnNumber() >= 0.

And let's change implementation accordingly.

63

In case of methods that return by value non-const overload is not needed.

65

Symmetry: errorLineNumber

66

Same here

68

Symmetry: errorColumnNumber

69

Same here

73

setErrorLineNumber (similar to what Qt uses in some XML or DOM APIs)

74

Symmetry: setErrorColumnNumber

77

KReportDesignReadingStatus would benefit from a copy ctor and operator=. Actually it's not that hard as the Private class is easily copyable because all its members are values.

src/common/KReportDesign_p.cpp
455

If we support KReportDesignReadingStatus::operator=() we could just do this here:

*status = KReportDesignReadingStatus();

And this would be more maintainable.

This revision now requires changes to proceed.Dec 20 2016, 9:27 PM
piggz marked 15 inline comments as done.Dec 20 2016, 9:58 PM
piggz updated this revision to Diff 9238.Dec 20 2016, 10:00 PM
piggz edited edge metadata.
  • Implement reveiew comments
staniek requested changes to this revision.Dec 20 2016, 10:21 PM
staniek edited edge metadata.

Good job. Almost there!

src/common/KReportDesign.cpp
53

It's enough to call *this = other

74

missing && d->errorColumnNumber >= 0

161

Unrelated to the port but while we're at this: we're not setting message at all here or removing previous one. Note that the status object is supplied by the user so we don't know what it is.

Proposed solution: if d->processDocument(doc, status) fails, set error message to something like tr("Error in XML document.").

src/common/KReportDesign.h
44

The return value needs to be KReportDesignReadingStatus&

This revision now requires changes to proceed.Dec 20 2016, 10:21 PM
piggz marked 4 inline comments as done.Dec 21 2016, 8:22 AM
piggz updated this revision to Diff 9246.Dec 21 2016, 8:28 AM
piggz edited edge metadata.
  • Address review comments and add a status or failure to process document
staniek requested changes to this revision.Dec 21 2016, 11:30 PM
staniek edited edge metadata.
staniek added inline comments.
src/common/KReportDesign.cpp
163

if (status) { .. }

This revision now requires changes to proceed.Dec 21 2016, 11:30 PM
piggz updated this revision to Diff 9284.Dec 22 2016, 8:20 AM
piggz edited edge metadata.
  • Only set status if it was supplied
staniek accepted this revision.Dec 22 2016, 10:16 AM
staniek edited edge metadata.

Good job!

This revision is now accepted and ready to land.Dec 22 2016, 10:16 AM
This revision was automatically updated to reflect the committed changes.
staniek reopened this revision.Dec 28 2016, 11:27 AM

@piggz I missed two things...

src/common/KReportData.h
46

Sorry, missed it, this method not needed.

48

Sorry, missed it, this method not needed.

This revision is now accepted and ready to land.Dec 28 2016, 11:27 AM
piggz marked 2 inline comments as done.Dec 30 2016, 2:10 PM
piggz closed this revision.Dec 30 2016, 3:22 PM