> Review: Needs Fixing none > Please review mono style guidelines, do not use _foo, m_foo, or any > syntax like that for member fields.
Done. > when checking if a string has contents, you should use > string.IsNullOrEmpty () Fixed > Services.cs:102; you pass a list with the ref keyword, it seems a bit > cleaner just to return a List object. What you have it looks like if > LoadServices is called more than once you'll get duplicates. I see > that you clear this before calling it in SystemItemSource, but that's > bad encapsulation, do the work in one place, don't split it up across > classes. Fixed > Services.cs:132; can you check if the string is in a blacklist before > you even add it to result? That should save you some work. > > You do a lot looping through lists to check if an element exists in > it, List has a Contains member function which will save you some > lines of code. Yes, I can. But I have many scripts in /etc/init.d/. If I use Contains (probably) all items in blacklist will be checked for each script. My method is much faster - I iterate over blacklist only once. > Do string.Format, not String.Format. Small style thing, this is C# > not Java Fixed -- https://code.launchpad.net/~karol-bedkowski/do-plugins/systemservices/+merge/2317 Your team GNOME Do Plugins Team is subscribed to branch lp:do-plugins/community-future. _______________________________________________ Mailing list: https://launchpad.net/~do-plugins Post to : [email protected] Unsubscribe : https://launchpad.net/~do-plugins More help : https://help.launchpad.net/ListHelp
