This is a effectively a review comment on your code based on an assumption that you used an AI coding assistant to create the code (though IME relatively inexperienced coders often copy and paste a lot of boiler plate code rather than finding a more readable and aesthetic means of achieving the same results).
But @derat does have a point here. These deeply nested if
structures are generally considered to be a fairly bad programming style which is difficult to read, easy to introduce bugs into etc.
So here are my code comments on this plugin:
- Again, well done for writing something that works and is useful.
- Please see my comments for your other plugin many of which are relevant as a response to this 2nd plugin as well.
- My biggest gripe is that this does NOT Show (or Hide) albums of a particular type as it describes itself, but instead it actually removes them from Picard (like Remove Perfect Albums does). TBH if it removes albums rather than hides / shows them, then I am not sure what benefit it really provides over and above Remove Perfect Albums (because the complete and unchanged / saved ones are typically going to be the majority of the albums you want to delete in most cases).
- I don’t think this plugin actually works the way I would expect it to. I am sure that you have tested it, and that it works the way you wrote it, however I would expect to open the popup (which would be initialised when Picard starts with all checkboxes checked), and by clicking on the checkboxes I would dynamically either show or hide particular types of albums from the tree. To achieve this you actually need to hook a callback to each checkbox, which then processes the tree when the value changes, and uses QT calls to hide or show the entries…
- It wouldn’t quite be the same functionality, but if you made it work this way, then you could have 2 checkboxes for Complete/Incomplete and Unchanged/changed which work in combination (plus a 3rd for error).
The big nested / indented code section has a few issues IMO:
6. I think you need to process all the albums in the tree, not just the selected ones.
7. The loop should be outside all the if
s, and the logic should probably be an a method of its own, and perhaps each option should call some sort of generic sub-method to avoid code duplication, perhaps passing a callable which can be used to check whether criteria are met.
So sorry, I think this plugin needs some more significant changes before it is ready to be included in the plugins list.
But don’t be disheartened - I think this can also be a useful plugin, but there is a bit more of a learning curve for you to climb before this is ready.