Extended SortedField with methods and a dptr
Details
- Reviewers
staniek - Commits
- R14:2209f06112fa: Add dptr to KReportData and SortedField
Ran kreportexample
Diff Detail
- Repository
- R14 KReport
- Branch
- dptr-final
- Lint
No Linters Available - Unit
No Unit Test Coverage
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. |
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& |
src/common/KReportDesign.cpp | ||
---|---|---|
163 | if (status) { .. } |