View Single Post
Old 07-25-2009, 05:08 PM   #453 (permalink)
KymikoLoco
Zewbie
 
Join Date: Jul 2009
Posts: 3
KymikoLoco is on a distinguished road
Exclamation Fail

(I apologize in advance for the programmer speak)

This type of program is exactly what I wanted for my Zune when I first opened it up, and realized it didn't come with the functionality of an alarm. I was happy to locate it, but quite irked with the fact that it wouldn't work no matter what I tried.

I first dl'ed the rar file, which, for whatever reason, deployed on my Zune, but almost immediately shut it down.... So then I dl'ed the actual game file, thinking I was doing it wrong. Yet again, just a shut down. So, being a good little programmer, I finally just got the source code, since the rar file only had the Program.cs file that called Run(), not the source that I needed.

Still, I couldn't run it, even making my own solution file in Visual Studio. I even dl'ed the newer version of XNA studio since I had Beta, so I now have 3.1. As I'm parsing through and debugging, trying to figure out WHERE it just shuts down, I notice the most annoying thing of all...

menuMain.Nodes.Add(new MenuSongLibrarySelectorItem("Play Music", menuMain, ref mainBlade));

You call this type of function... the new MenuSongLibrarySelectorItem, 4 times. As far as I can tell, it does the same thing, in 2 different constructors. Now, just because the Microsoft Framework has a garbage collector, that does NOT mean you call can "new" 10 + 6*(NumOfPlaylists * (2 * NumSongsInPL) + .... I don't care to do the math anymore.... So, if I have 0 playlists, and 1 artist with one song then you call "new" 13 times in one function. Multiply that by 4, and you get AT LEAST 52 calls to new ON INITIALIZE, for just one song! And, given the fact that the library will NOT change 4 separate times in rapid succession, my advice is to rewrite your "new MenuSongLibrarySelectorItem" to check to see if you have already loaded the library once, and then if it does, add to the menu selectors what has already been loaded in. Like, a static library or an actual member of the ZuneAlarm class, and load the library on Init, and use that for the selectors.

Don't take this as an attack, because that is certainly not my intention. If you write it to not be so memory intensive, it will save on your load times, and won't take up whatever RAM the Zune has to offer you.

I suggest reading this article, and running it on your Zune.
How To: Monitor Performance at Run Time
I ran it a few minutes ago, and within the first 20 seconds, the Garbage Collector ran 11 times, and cleaned up over 7megs (a lot for ANY program). This literally hurts my brain to think that I ran this on my Zune. No wonder he shut down every time.

As one programmer to another, you should probably take this down, because it is poorly structured for a C# program.

(EDIT) - I just noticed you load the library once, but you call new for the menus, which is still bad, since you can reference just one EventHandler instead of making a new one for each.... everything. Since each EventHandler gets different arguments/objects as the sender. Which is the beauty of the object class... it can be treated as anything Have them reference an event you create, instead of letting C# do it for you. It's more work, but it saves SOOOO much memory.

And your volume controls are.... convoluted. setVolume's params do nothing. It's an unnecessary function.

Last edited by KymikoLoco; 07-25-2009 at 05:30 PM. Reason: Noticed library



KymikoLoco is offline   Reply With Quote