Dead Code Must Be Removed

Here, various developers can tell you all about what they're up to, so you can yell at them for being idiots. "... and there was a great rejoicing."
Post Reply
Message
Author
User avatar
Cjkjvfnby
AI Contributor
Posts: 539
Joined: Tue Jun 24, 2014 9:55 pm

Dead Code Must Be Removed

#1 Post by Cjkjvfnby »

There is a good article about dead code and its consequences https://www.infoq.com/news/2017/02/dead-code/
Deleting dead code is not a technical problem; it is a problem of mindset and culture
It took me some time to change my mindset and start removing dead and commented code.

I have created this topic to share my view on development and to be on the same page with others.
For Python code I have already used all easy ways to find dead code and remove it.
I don't know a situation in C++ part but want to encourage someone to make a look at it and get low hanging fruit. It is not only about code but also about build scripts and dependencies.

I don't know about tools for static analysis of the C++ but read some articles about https://www.viva64.com/en/w/ and it looks like a possible solution: it has trial and free license for open source projects and dead code analysis.
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13586
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Dead Code Must Be Removed

#2 Post by Geoff the Medio »

Cjkjvfnby wrote: Tue Jan 12, 2021 8:02 pm I don't know a situation in C++ part but want to encourage someone to make a look at it and get low hanging fruit.
Visual Studio has some built-in code analysis that I've used to find potential issues in FreeOrion code, but the results have been mostly unimportnant or arising from external, mostly Boost, code.

User avatar
Cjkjvfnby
AI Contributor
Posts: 539
Joined: Tue Jun 24, 2014 9:55 pm

Re: Dead Code Must Be Removed

#3 Post by Cjkjvfnby »

Geoff the Medio wrote: Wed Jan 13, 2021 10:28 pm Visual Studio has some built-in code analysis that I've used to find potential issues in FreeOrion code, but the results have been mostly unimportnant or arising from external, mostly Boost, code.
Linters for AI have very flexible configuration, you can disable specific warnings for a project or per file. I don't like to add any suppression warnings to code because it pollutes it.

Looks like PSV has that kind of config too. https://www.viva64.com/en/m/0017/#ID7DBBA508CB

So if you could spend some time investigating if the project could benefit from the static analysis it would be great.

I'm really happy with linters in Python (actually I use much stricter rules in my configs) this helps to set a minimum quality level and maintain it without effort. The most important thing that this doesn't depend on the coder's experience. I took part in the mentoring program in my company, and review homework for 4 students (~3-8 programming tasks per week for each one). It tooks me some time to set up and explain the idea and almost zero effort later.
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

User avatar
em3
Vacuum Dragon
Posts: 630
Joined: Sun Sep 25, 2011 2:51 pm

Re: Dead Code Must Be Removed

#4 Post by em3 »

The Clang compiler has some very robust tools for detecting unused and unreachable code.
https://github.com/mmoderau
[...] for Man has earned his right to hold this planet against all comers, by virtue of occasionally producing someone totally batshit insane. - Randall Munroe, title text to xkcd #556

User avatar
Cjkjvfnby
AI Contributor
Posts: 539
Joined: Tue Jun 24, 2014 9:55 pm

Re: Dead Code Must Be Removed

#5 Post by Cjkjvfnby »

There is a pragmatic approach to development: add code when you need it. That means that every code you add has a reason.

What could happen if you do not follow this code? Wasted time. Someone adds code, someone refactoring, someone finds out that it is not used/broken, someone removes it. Time and resources are wasted.

Any code takes it's tall: it increase the time to compile, it makes you scroll more.

Here is an example:
https://github.com/freeorion/freeorion/pull/1763/files

2 classes Map and Vector added and only a single method to access data was introduced. So vector was not accessible to AI from the beginning.

The Freeorion approach is to introduce a new feature and add it to the Python wrapper just in case. I want to change this approach: let's add new features to AI interface when it is requested by AI team.
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13586
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Dead Code Must Be Removed

#6 Post by Geoff the Medio »

The Freeorion approach is to introduce a new feature and add it to the Python wrapper just in case. I want to change this approach: let's add new features to AI interface when it is requested by AI team.
Problems I see with that are that I wouldn't in general assume you or other AI people know what info is or could be made available via the Python API, so you wouldn't generally know what you can or should ask to have added to the API. Analogy: a colourblind painter not realizing that blue paint is available and just using red and yellow and black. Also, if the API doesn't provide info before it's wanted, then when an AI coder does want it, they have to wait for someone to add it to the API instead of immediately having access to that info. It's also easier for me to add stuff to the API when it's implemented in C++ than to come back later and figure out again what stuff should be added.
Cjkjvfnby wrote: Sat Feb 06, 2021 10:43 amHere is an example:
https://github.com/freeorion/freeorion/pull/1763/files

2 classes Map and Vector added and only a single method to access data was introduced. So vector was not accessible to AI from the beginning.
To me, the appropriate response would be for an AI person to post a bug that the API doesn't actually provide useful access to that info, even though it appears to intend to.

User avatar
Cjkjvfnby
AI Contributor
Posts: 539
Joined: Tue Jun 24, 2014 9:55 pm

Re: Dead Code Must Be Removed

#7 Post by Cjkjvfnby »

so you wouldn't generally know what you can or should ask to have added to the API
The main driver of AI is a player. If the player says that AI does something stupid we start acting. For example, AI does not use some new features.
If we have a problem we can think of what should be done and how C++ binding could help us.
Also, if the API doesn't provide info before it's wanted, then when an AI coder does want it, they have to wait for someone to add it to the API instead of immediately having access to that info.
And sometimes they still want to add, because the person who added these features did not guess how they should be used.

It is comfortable to work with the code when you know: features in the code work. Usually having good test coverage helps. In the case of AI binding, sometimes this code has never been run.
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13586
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Dead Code Must Be Removed

#8 Post by Geoff the Medio »

Cjkjvfnby wrote: Sat Feb 06, 2021 1:19 pmAnd sometimes they still want to add, because the person who added these features did not guess how they should be used.
Sure... Having a feature be advertized doesn't mean it's optimally implemented.
It is comfortable to work with the code when you know: features in the code work. Usually having good test coverage helps. In the case of AI binding, sometimes this code has never been run.
Can we improve the test coverage and how can tests of new API features be best added?

Post Reply