Do

Code review comment for lp://staging/~jassmith/do/trunk

Revision history for this message
David Siegel (djsiegel-deactivatedaccount) wrote :

Why do you have IDockItem and AbstractDockItem -- why not just abstract class DockItem?

### AbstractDockItem

  Do not allow any characters but tabs before get { ...

  Do you need to cast to int?
  (int) ((DockPreferences.FullIconSize - pbuf.Width) / 2

  Why is GetDragPixbuf returning null? That seems bad. Maybe make the method abstract?

### ApplicationDockItem.cs

Please organize your using statements at the top of your files:

   using System;
   using System.Collections.Generic;
   using System.IO;
  +using System.Linq;

Your summary for GetSurfacePixbuf is too long. Put remarks in <remarks> tags. Summary should be short and to the point (1 sentence, maybe 2).

GetSurfacePixbuf is way too long and ugly. Break it up into smaller methods, use syntactic sugar to avoid repeating yourself.

Holy moly, please do not look for DesktopFiles. Why are you doing this? Don't do it.

Import System.IO, import System.Diagnostics. Drop the prefixes.

WindowCount is an expensive call, should be GetWindowCount ().

You have some double newlines.

new Wnck.Application[] {application} can be new [] { application }.

Why is your property "App"? It should be "Application".

GetMenuItems is really messy. Make it sexy. You have literal constants. Move them to consts. Use yield. Use camelCase for locals.

The body of Equals should be:
  return other is ApplicationDockItem && Equals (other as ApplicationDockItem);
Then implement IEquatable<ApplicationDockItem>. Also, do you declare IDockItem to be IEquatable<IDockItem>?

I stopped here. ApplicationDockItem needs a lot of cleaning up. Let me know when it's ready to be reviewed again.

« Back to merge proposal