> 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.

Probably, although priv is just a private member with implementation details, I don't really see this as a large design problem.

>
> 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 ()"; }"

> OK, I'm still wading through the changes. But I feel like a rant...border () * 2,border () * 2);geometry.width () + priv->geometry.border ()height () + priv->geometry.border () * 2);geometry.width ()" imply too muck knowledge

>

> 1307 CompSize

> 1308 CompWindow::size () const

> 1309 {

> 1310 - return CompSize (priv->width + priv->geometry.

> 1311 - priv->height + priv->geometry.

> 1312 + return CompSize (priv->

> * 2,

> 1313 + priv->geometry.

> 1314 }

>

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

>

> 1. Chained operations like "priv->

> of the details of "priv".

> That is "priv->width ()" would reflect less coupling.

Probably, although priv is just a private member with implementation details, I don't really see this as a large design problem.

>priv->geometry)" would:Geometry:CompWindow::Geometrypriv->geometry); }"

> 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(

> reflect better coherence.

>

> So, assuming (because borders may be optional?) that there's an unambiguous

> mapping from CompWindow:

>

> a. Add a constructor to CompSize: "explicit CompSize(

> const& g) : mWidth(g.width () + g.border () * 2) ..."

> b. Add an inline method "CompSize PrivateWindow::size () const { return

> CompSize(

> c. Rewrite the above as "CompSize CompWindow::size () const { "return

> priv->size ()"; }"

+1 for all three