Code review comment for lp:~smspillaz/compiz-core/compiz-core.work_923683

OK, I'm still wading through the changes. But I feel like a rant...

1307 CompSize
1308 CompWindow::size () const
1309 {
1310 - return CompSize (priv->width + priv->geometry.border () * 2,
1311 - priv->height + priv->geometry.border () * 2);
1312 + return CompSize (priv->geometry.width () + priv->geometry.border () * 2,
1313 + priv->geometry.height () + priv->geometry.border () * 2);
1314 }

As a change this doesn't look too bad - but it ignores a horrid design!

1. Chained operations like "priv->geometry.width ()" imply too muck knowledge of the details of "priv".
   That is "priv->width ()" would reflect less coupling.

2. You're tracing the path "priv->geometry..." many times, which suggests that the logic belongs in "geometry".
   "return priv->geometry.size ()" or "return CompSize(priv->geometry)" would reflect better coherence.

So, assuming (because borders may be optional?) that there's an unambiguous mapping from CompWindow::Geometry:

a. Add a constructor to CompSize: "explicit CompSize(CompWindow::Geometry const& g) : mWidth(g.width () + g.border () * 2) ..."
b. Add an inline method "CompSize PrivateWindow::size () const { return CompSize(priv->geometry); }"
c. Rewrite the above as "CompSize CompWindow::size () const { "return priv->size ()"; }"