“…
As I mentioned in above post there is one major drawback in current GUI code. The current code does not follow Qt:s Model-View architecture. In practice we store the error data directly to the View and just use default standard model. But it should be other way around - data is stored to Model and we should use the default View code. This severely limits what we can do (efficiently) in GUI currently. This must be fixed, no way around it.
…”
I got into the mood once more to adjust this software situation a bit. The source code structure was extended so that the member variable “mModel” could be deleted from the class “ResultsTree” finally.
The implementation contains still some open issues for further development considerations.
I have not worked with a database for at least 10 years. I only have basic knowledge about databases. I can write a SQL query that will list all data in a TABLE for instance.
A list for the results works very well. I see no problem with that.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Did you notice a link to my intermediate development results?
Thanks! I have taken a quick look at those changes. I assume it's work-in-progress so I will not comment on the details yet. But before this is merged you will need to:
* Follow our naming conventions
* Don't overuse/abuse smart pointers, if you can use a normal local variable then do that.
But well.. I think it looks promising in general. This looks like code I could maintain.
Last edit: Daniel Marjamäki 2018-09-23
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
"ei", "rm" etc. abbreviations in variable names should in general be avoided.
I try to avoid the repetition of information which should be provided by the data type there.
but in general you should use signals/slots rather than callbacks in Qt code.
Is the usage of callback functions appropriate when the data model needs to check something for an action for which the corresponding (tree) view manages required display data?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I would prefer the other naming style over camel case usage.
I am not surprised. We have a coding convention and of course it does not feel natural to everybody. No coding convention will please everybody.
Just follow it and ensure that your code is consistent. I will not even try to read and understand it until it is consistent.
Is the usage of callback functions appropriate when the data model needs to check something for an action for which the corresponding (tree) view manages required display data?
I do not know. When your code is more readable for me, I can look at it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
most of the coding style is enforced/fixed by the runastyle script (indentations, where you put {}, etc). the runastyle script is the documentation.
the naming conventions are not enforced by the runastyle script. You should try to choose names that are consistent with other names in Cppcheck. If you look around a little you can for instance see that functions names use camelCase. That type names start with uppercase. That members start with "m" etc..
Feel free to write a document and add it in a PR.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
the naming conventions are not enforced by the runastyle script.
It seems that we need to consider additional communication challenges for the desired source code structures then.
I am also curious about the clarification if any of the shown change possibilities should be split into smaller update steps. Consensus should be achieved for a patch order.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I am also curious about the clarification if any of the shown change possibilities should be split into smaller update steps. Consensus should be achieved for a patch order.
I would prefer to see some small refactoring PRs first. For instance, you spotted a case where we used new, this was a good catch. And the code can be rewritten so no new is used.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Kimmo Varis contributed the following details for the discussion topic “GUI: Ideas to improve?” on 2011-07-17.
“…
As I mentioned in above post there is one major drawback in current GUI code. The current code does not follow Qt:s Model-View architecture. In practice we store the error data directly to the View and just use default standard model. But it should be other way around - data is stored to Model and we should use the default View code. This severely limits what we can do (efficiently) in GUI currently. This must be fixed, no way around it.
…”
I got into the mood once more to adjust this software situation a bit. The source code structure was extended so that the member variable “mModel” could be deleted from the class “ResultsTree” finally.
The implementation contains still some open issues for further development considerations.
See also:
- Commit b39c15410bb853d69f3d44953e6eaf6220aed491 (by Vesa Pikki on 2009-03-01)
- Extensions around data models and views
Now I am curious on how the clarification of the remaining issues will evolve.
What are the open issues? I am curious to see your code.
Did you notice a link to my intermediate development results?
I am struggling also with some software design constraints around the Qt class library.
But I got another development idea.
How do you think about to work with a database for the presentation of source code analysis results?
Can you accept a graphical user interface which will work with an in-memory database?
I see no reason to turn the list of results into a database.
I have not worked with a database for at least 10 years. I only have basic knowledge about databases. I can write a SQL query that will list all data in a TABLE for instance.
A list for the results works very well. I see no problem with that.
It is good to know that you are familiar with such software to some degree.
I imported information from Linux source code into database tables a few times.
How do you think about to clarify remaining data modelling challenges in more detail?
Thanks! I have taken a quick look at those changes. I assume it's work-in-progress so I will not comment on the details yet. But before this is merged you will need to:
* Follow our naming conventions
* Don't overuse/abuse smart pointers, if you can use a normal local variable then do that.
But well.. I think it looks promising in general. This looks like code I could maintain.
Last edit: Daniel Marjamäki 2018-09-23
Yes, partly.
Please provide more constructive feedback for this clarification request.
Which naming approach would you prefer here?
I am using them on purpose. Where do you get the impression of “abuse”?
Will any additional software adjustments become feasible?
please rename "do_stuff" to "doStuff". "file_name" to "fileName". etc..
I think variable names are cryptic. "ei", "rm" etc. abbreviations in variable names should in general be avoided. especially for non-local variables.
I did not investigate entirely.. but in general you should use signals/slots rather than callbacks in Qt code.
Last edit: Daniel Marjamäki 2018-09-24
I would prefer the other naming style over camel case usage.
I got used to shorter identifiers.
I try to avoid the repetition of information which should be provided by the data type there.
Is the usage of callback functions appropriate when the data model needs to check something for an action for which the corresponding (tree) view manages required display data?
I am not surprised. We have a coding convention and of course it does not feel natural to everybody. No coding convention will please everybody.
Just follow it and ensure that your code is consistent. I will not even try to read and understand it until it is consistent.
I do not know. When your code is more readable for me, I can look at it.
Does an official document exist for the required coding style?
no :-(
most of the coding style is enforced/fixed by the runastyle script (indentations, where you put {}, etc). the runastyle script is the documentation.
the naming conventions are not enforced by the runastyle script. You should try to choose names that are consistent with other names in Cppcheck. If you look around a little you can for instance see that functions names use camelCase. That type names start with uppercase. That members start with "m" etc..
Feel free to write a document and add it in a PR.
I would prefer to see some small refactoring PRs first. For instance, you spotted a case where we used new, this was a good catch. And the code can be rewritten so no new is used.
Is a clean-up of GUI source code required before any more meaningful software adjustments will be considered once more?
Yes. I reviewed your previous PR. And I saw no action. We just got into fruitless discussions. So it was waste of time.
You spotted a piece of code that uses new wrongly (good catch!). I would be quite interested to see a specific fix for that.
I see no reason to clump that change into another large PR. I want that code to be fixed and I don't want it to be blocked by other diffs.
Further bug reports might follow in this forum then.
I offered additional ideas and some source code adjustments.
We came along topics where we have got still different software development opinions.
Do you expect the registration of tickets in the Trac system for remaining update candidates?
It was fruitless.
We had disagreements and debated about that.
That took valueable time. And that is a cost.
No code has been changed. So there is no gain.
To me you are benevolent troll. We loose lots of time in debates and then no code is ever changed.
Communication needs some time to resolve disagreements, doesn't it?
Usual development challenges can be interpreted also in this way.
This can be also usual when no consensus was achieved for a change possibility so far.
You might reconsider this view.
I find this information inappropriate. The software is evolving together with its contributors through the years.
I see no results. We can debate about something and if we eventually resolve a disagreement. you do not send a PR. So why debating in the first place?
I see very few contributions from you.