← Back to team overview

do-plugins team mailing list archive

Re: [Merge] lp:~karol-bedkowski/do-plugins/systemservices into lp:do-plugins/community-future

 

Review: Needs Fixing none
Please review mono style guidelines, do not use _foo, m_foo, or any syntax like that for member fields.

when checking if a string has contents, you should use string.IsNullOrEmpty ()

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.

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.

Do string.Format, not String.Format. Small style thing, this is C# not Java.
That's all for now!
-- 
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.



Follow ups

References