search

kitodo / kitodo-production

Kitodo.Production is a workflow management tool for mass digitization and is part of the Kitodo Digital Library Suite.
http://www.kitodo.org/software/kitodoproduction/
GNU General Public License v3.0
64 stars 63 forks source link

Suppress code style warnings for some long methods #6146

Closed stweil closed 3 months ago

henning-gerhardt commented 3 months ago

I agree with @solth here: suppressing warnings is not a good way even it is already done in the past for other warnings. Suppressing warnings hide them in the IDE but the code did not work better. In my opinion the warnings should stay visible so that everyone can see and may correct them.

stweil commented 3 months ago

The warnings exist for years now and nobody cared and fixed them. And the current code contains more than 200 @SuppressWarnings statements. For the three methods I see no need to split them just to avoid a codestyle warning. Therefore I think suppressing these warnings is the best choice. If you think that the functions are too large and that they should be split, I can offer to create an issue for that.

Ideally in my opinion a successfull build should not show any warning or error messages. The current build is far away from this, and this PR is a small step to reduce the number of unnecessary warnings.

solth commented 3 months ago

@stweil I disagree. IMO, a long method is nearly always difficult to maintain, the three examples in this case are no exception. Refactoring them is always preferable to ignoring the corresponding warnings. As @henning-gerhardt mentioned, just the fact that we already have a lot of suppressed warnings - and many more issues - in the code doesn't justify repeating the same error again and suppressing even more of them.

If our only goal was to have a clean build without any warnings we could suppress all current code style warnings, or even easier, disable code checks completely. Conversely, opening a new issue for each warning is overkill. These warnings are clearly visible in the build and code checker logs (the reason why you became aware of them in the first place).

I will close this pull request as I do not think it improves the code or software.