mmc4w.py Use a dictionary of arrays for Vol+ and Vol- button colors

[donburch888]

I like the way your volume buttons change color as a quick visual clue to volume, and I noticed a long list of IF statements setting up the colours of the Volume buttons to make this happen.

After using a dictionary for buttons on my own adaptation; I thought this could be simplified.
Result is a dictionary with key being the volume level, and containing an array with all the button definitions can be defined; and then only 3 lines of code to process the colour changes.

[drgerg]

Thanks, Don! I like what you did there. You are the very first person to ever offer code to improve one of my projects. I think that's pretty cool, and now I have to figure out what to do. I will look into that over the next few days. I still work full-time, and have some other pressing projects in the works, but I'll get to this soon. Thanks again!

[donburch888]

Greg, its the first Pull Request i've done too ;-)
30 years as a professional programmer; but away for over 10 years, so new to python, venv and github.

I know what it's like, diving in to quickly build a program; then finding yourself deep in the details and adding more and more code ... whereas when maintaining someone else's program I find myself thinking "why did they do it this way?" and wanting to simplify/reduce the code to make it easier to see the big picture.

I made numerous small tidy-ups ... checking for fields like "Album" that arent tagged in my music files, removing a few variables and functions which are used only once or twice ... but my main focus is on a user interface thats opposite to your (totally valid) use case.

[drgerg]

Don, If I'm reading this correctly, you're saying make 'confparse' global and don't read the disk file so often.

How certain are you that the in-memory version of confparse carries any and all changes since the last read or write? I'm not sure it does. But I could be learning something here.

[drgerg]

. . . I suppose that does make sense, in that the changes are written to the confparse instance before we write them to the .ini file. I guess I wasn't sure I could trust that what was in memory reflected what was written to the .ini file.

That may not make sense, but not everything I do makes sense, so no surprises here.

Everything else looks like a useful contribution to me.

[donburch888]

You are not wrong ... it is better to program defensively by not making assumptions ... and your program is ensuring it always has the latest information from the file. :-) Also in theory each function should be self-contained to make it easier to understand. Thus we should pass values as function parameters - to better document what is being passed in and out - and should minimise the number of global variables, except where they really do apply to almost all functions.

On the other hand, the latest information was updated in the .ini file by your program; and I assume that no other program or user will be making changes to the .ini file while the program is running. Thus your program already knows the latest information and doesn't need to keep re-reading it. A potential problem comes if you have multiple variables containing the same (or very similar) data, and you get confused and use the wrong one (which I have done on occasion).

I suggest that confparse and cp are right on the border of "do I or don't I ?". If you are going to come back and update the program it may be worthwhile looking into - but the program works well as is.

You should also consider that my code is now very different from yours - I have stripped out the menus and pop-up windows, instead using lots of buttons to start playlists running on a dedicated music player with 7" touch screen - so I may well have missed something important in your code. You should definitely think twice (and make a backup) if you do imlement that suggestion.

[drgerg]

Thanks for the input. I am planning to get back into MMC4W a bit later in the year. When I do, I'm going to steal some of what you did here. I think (for now) I'll leave confparse alone, or maybe I'll run some tests if I have time and learn from the testing.

Right now all my time is sucked up with work and my cargo trailer conversion project. My blog is behind on that project because there are only so many hours in the day. I'm normally not this hectic, but the hurricane forecast earlier this year lit a fire under my ass and now I'm on a mission to get "The Bug" done.

BTW: I really liked your two lines to round numbers to 5. I think I've seen similar, but at the time I could not think of it. I don't see things mathematically, so I had to take my pedantically linear approach. It does work, but yours is more elegant. Thanks!

[drgerg]

Hey Don, I manually included your fiver() replacement. I have not yet committed my changes, but I thought I'd say thanks again for the suggestion. It is much better.