Maintainability OpenCPN

[wernertoonk]

I searched this forum and the developer manual for any remarks regarding Technical Code Quality (i.e. Maintainability), coding guidelines and static code analysis. I noticed Codacy as tooling, though not much information.

I am a huge fan of the 10 practical guidelines to write maintainable-software. Perhaps because I worked at the company that published the theory and the book. But also because I've seen these guidelines do work. See here:
https://bobbelderbos.com/2016/03/bui...able-software/

SIG also offers tooling that fully integrates with GitHub, support C++ and is free for OpenSource. Very useful.

I just forked the OpenCPN code base, and ran the analysis (with the default configuration). Score is 2 out of 10. Too many units that are too long and too complex (therefore unable to test automatically), too much redundancy, too many parameters, too few separation of concerns, too big code base, etc.

First analysis says 34 person-year of effort.

Redundancy: jsonval.cpp (1406 lines is 4 times in the code base), gl.h and private.h share 3 times 527 identical Lines of Code (LoC) etc.

glIndexCall for exampel has 12.455 LoC with 1.748 Branch Points, CanvasPopupMenu as 499 LoC and 171 Branch Points.

Etc.

Not sure if I can share the results easily, though more that willing to manually export the lists.

What I am curious to find out, if there is anyone working on improving maintainability, measuring maintainability and striving to improve on this.

The higher the maintainability is, the easier it is to find and fix bugs, to add functionality, to reuse components, etc. etc. etc. Anyone here to respond on this?

[CarCode]

That is a known issue. The OpenCPN code was done having obfuscation in mind and not maintainability.

[transmitterdan]

Don’t listen to someone who does not contribute anything to the code.

The code started many years before such standards existed. It is not done by paid professionals either. So worrying about compliance with a standard that did not exist when the paper was blank is no of great value.

Would the project benefit from cleaning up redundant code sections and improved readability. Yes, it definitely would. What would be of the most value IMO would be some data flow and process flow diagrams. That would make troubleshooting much easier and would give new contributors some context for how it all works.

I am sure anyone willing to contribute documentation or to make improvements in the code for better standards compliance would be welcome. But berating the original authors for not following any then nonexistent standard will not bear any fruit, again IMO. They didn’t have a detailed product specification, coding standards manuals, data flow diagrams, 5 software architects to keep all that stuff current and up to date.

They wanted a tool for their boating that worked well on a PC and didn’t cost an arm and a leg. From that simple product requirement they have been wildly successful.

I estimate it would take 50 man years to bring the code into compliance assuming no new features are added in the mean time. By that point no one will be around to care unless a large team works hard to bring about the changes rapidly. Also needed would
be a small army of regression testers as well.

[wernertoonk]

Thx. Let's start by making clear that I am very happy that the original coders started this great project and all other coders after that spend time and energy on creating all the functionality that there is now. As said: happy and very thankful! I've been using it for years and years, and sailed half the world with it, (the other half will follow).

And I am perfectly aware of how such a coding project starts and what it leads to, if there are no guidelines or checks on that. I've analysed quite some of these codebases and OpenCPN ain't that special. Standards regarding maintainability on the other hand exist for over a decade already, and one doesn't have to be a professional to adhere to some basic guidelines.

But please, I do hope you're not referring to my question with 'berating'. I am trying to contribute to the code (first pull request is out), and I do notice that maintainability of the codebase is low. That is a fact, not an opinion. What I did was sharing the facts and the outcome of the measurements.

IMHO we could do ourselves a favour and define some coding guidelines, and get some appropriate tooling in place and make sure maintainability doesn't drop any further. I can see we tried to do some work on quality, with Codacy. That particular tooling however, does not even (yet) calculate cyclomatic complexity. So, for true code quality measurements here it ain't useful (enough).

We can't just keep on glueing if-then-elses in modules that are way too long to test automatically already.... By continuing to do so, it will become harder and harder to maintain the code, and to get developers that are new to the codebase up to speed. That's a pity that hinders us all.

I am not a huge fan either of completely rewriting , and spending 30 - 50 person years on it. Taking measurements to prevent maintainability to drop maintainability any further would be very wise to do. By applying the boy-scout rule (leave the campground better than you found it), we should b be able to gradually improve maintainability. That is, ... if we want that.

More than willing to discuss that any further and to contribute to that. Looking forward to more response.

[transmitterdan]

Werner,

I don’t disagree that the code is long and complex. I would support a PR to define coding standards to be used going forward. Even better would be additions to the CI process to automate code review of new or changed functions. If an edited function or class fails some automated metric then it becomes a candidate for review and rewrite. Over time the code will improve.

But you are right, it would be a complete waste of time to stop all new features and rewrite the whole thing against a modern coding standard. By the time that job is finished all users would have moved on (or stop breathing).

In the moment there are some big features in the works that users have been clamoring for a while. No user is complaining that the source code is hard to read/understand.

Any known errors such as null pointer, memory leaks that grow indefinitely, etc. will be fixed as soon as someone points them out. I have been running O on a raspberry with 1GB of ram. It’s been running continuously for weeks with no growth in used memory. So I don’t believe there are huge unknown bugs.

FWIW this project started way more
than 10 years ago, long before any of us even knew about it.

[wernertoonk]

Thx again. Precisely. Coding standards and automated code review. If edited code fails the metric, refactor. We could have started with that 10 years ago, or even earlier. Whereas the tooling now is far better. And indeed, users don't complain about code quality, though they will notice the effects since new functionality takes longer then necessary, testing requires more effort, etc. :-) Let's start.

[seandepagnier]

I would like to see opencpn separated into different modules, and eliminate many required dependencies such as wxwidgets.

In this way opencpn could have different build setups and ui backends. It could run on webservers to provide charts rendering on website. It could run in a mobile ui designed for touch with gestures. It could also retain the original wxwidgets ui.

[rgleason]

I agree with TransmitterDan & Sean and your ideas:

1. Taking measurements to prevent maintainability to drop maintainability any further would be very wise to do.
2. By applying the boy-scout rule (leave the campground better than you found it), we should b be able to gradually improve maintainability. That is, ... if we want that.
3. ..Define some coding guidelines, and get some appropriate tooling in place and make sure maintainability doesn't drop any further.

Are there good free/opensource software tools that code check to identify the most problematic areas and types of problems? Could you identify a such a good toolset and set them up to identify the most fruitful areas for OpenCPN improvement?

Incrementally working on specific problematic areas or improving maintainability of sections requiring new work are some good ways to make a difference.

Plugin authors could certainly use some guidance about which tools are best to use, and some guidance and reading to understand these concerns. If you would like to start a "Maintainability" page in the Dev Manual, that might be a good start.

For example Alec Leamas has done some great work for OpenCPN, improving the processing and deployment of multiple OS and then building a Plugin Installer. It has taken a number of months and great persistence, but it has been a significant improvement.