Code review comment for lp:~jassmith/do/docky-random-polish-and-breakage

DockServices.cs
 Does this class need to be static? Have you thought about using the existing
  services class instead of duplicating it here? You can just use IService,
  IInitializedService instead of reinventing the wheel.
 Use a static constructor to initialize fields.
 Please don't use the E1 ?? (E1 = E2) form; use if (E1 == null)...
 For items_service, drawing_service, do_interop_service, just new those in the
  static constructor and use auto-properties.
 For LoadService, you should use OfType<TService>.FirstOrDefault (), then check
 for null and use the Activator if you need it. Or get fancy with ??

DoInteropService.cs
 You may want to use EventHandler and pass (this, EventArgs.Empty) instead of
 using Action.

Default.DrawingService.cs
 Looks good.

Default.ItemsService.cs
 Can you make your fields more generic -- IDictionary instead of Dictionary,
  ICollection instead of List.
 s/Dragable/Draggable

 ItemCanBeMoved (int item)
  return DockItems [item] is DockItem && ItemCanBeMoved (DockItems [item]);
 ItemCanBeMoved (DockItem item)
  return DraggableItems.Contains (item);
 It's very weird that you case DockItems [item] to DockItem.

 DropItemOnPosition, MoveItemToPosition:
  Don't repeat yourself. Make the following its own method:
  if (DockItems.Contains (item) && 0 <= position && position < DockItems.Count)
 MoveItemToPostion (int, int) doesn't do upper bounds checking.

 DockPreferences.AutomaticIcons--
  is a bit scary

 Enumerable.ToDictionary throws a not implemented exception on mono < 2.0

 You should writegeneric Serialize<T> and Deserialize<T> methods instead of
 repeating yourself. You can overload them with Funcs if you want to alter the
 datastructure before/after serialization, which you seem to be doing.

 UpdateWindowItems
  first foreach, you have a line ending with '.'

 SimplifyPositions:
  s/i=0/i = 0
  Why do you call ToArray?

Default.PainterService.cs:
 Bad file name in header comment.
 Use Log<PainterService>

IItemsService.cs
 Don't use List, use the thinnest interface you can -- IEnumerable is best.
 It's very weird to have a readonly property for a mutable type.

 You should not have overloads in your interface. Do calling classes really use
 both versions of MoveItemToPosition and RemoveItem? That may be a sign of a
 poor design decisison.

 A few comments would be nice!

IPainterService.cs
 Wrong filename in heaer comment

ApplicationDockItem.cs
 DesktopFilesDirectories should be static readonly IEnumerable<string>, not a
 property.

BaseDockItem.cs
 MakeIconSurface
  if (icon_surface == null)
   icon_surface = ...
  return icon_surface;

DoDockItem.cs
 const "Summon GNOME Do" at the top of the file

DoInteropService
 Why do you have IDoInteropService and DoInteropService?

DockArea.cs:
 BaseDockItem needs to be public?
 Magic numbers 22, 300, 500?

 Your AnimationStateConditions could hav better indentation (I still think the
 string-based criteria are a bit wierd, would prefer enum). Now that I read
 further and see you writing down "PainterAnimation" over and over again, I
 *really* think you should be using an enum.

 You frequently place an unneeded ( ) around the predicate in your ternary
 expressions.

 HideInputInterface
  Put the new Gdk.Rectangle on its own line. Can you factor out the
  window.SetInputMask call so it only happens once? Maybe make a method that,
  given an orientationm, returns a Gdk.Rectangle.

DockArea_DragAndDrop.cs
 Magic number 8?

DockArea_Rendering.cs
 VerticalOffset
  Your multiline expressions could use some better indentation. See "offset =
  Math.Min ..."

 Magic number 1.5?

 IconAnimation
  Fix that ternary expression.

DockState.cs
 Just make the Instance public readonly DockState = new DockState ();

DockWindow.cs
 s/interopService/interop_service/g

 You may want to create utility functions:
  Util.ForOrientation (Action top, Action right, Action bottom, Action left)
  Util.ForOrientation<T> (Func<T> top, Func<T> right...
 Because you repeat the switch (DockPreferences.Orientation) pattern a lot.
 Here's an example (there isn't Gtk.Point, so you can omit Gdk, right?)

  position = Util.ForOrientation (
   () => new Point (x - req.Width / 2, y),
   () => new Point (x - req.Width, y - req.Height / 2),
   () => new Point (x - req.Width / 2, y - req.Height),
   () => new Point (x, y - req.Height / 2)
  );

DockBackgroundRenderer.cs
 Lots of magic numbers here.

DragState.cs
 Can this be a struct? Please don't use public fields either way.

PaintNeededArgs.cs
 Magic number 15

PainterService.cs
 Use ICollection instead of List if possible.

SeparatorItem.cs
 Very gross arithmetic expressions in here.
 Are you using hashcode equality to do reference equality checking? Use
 object.ReferenceEquals for this.

Docky.Interface.Util.cs
 You use static ints, should be const.
 Use camelCase for parameter names.
 Lots of magic numbers in here.

WindowUtils.cs
 It's scary in here.
 Can BadPrefixes be static readonly IEnumerable?