[Cuis-dev] Theme>>useUniformColors considered harmful

[Luciano Notarfrancesco]

Sounds very good to me. And while you’re at it, perhaps we can remove
useTaskbar as you suggested before, as long as we avoid randomly showing
the taskbar again. Juan already removed the call to restoreDefaultDesktop,
and we should also avoid adding the taskbar again in Theme
class>>currentTheme:

On Mon, 25 May 2020 at 5:33 PM, Casey Ransberger via Cuis-dev <
cuis-dev at lists.cuis.st> wrote:

> I’m poking around in the vicinity of Theme, thinking about adding a class
> to Theme-Core. When I initially wrote Theme, I did a lot of gymnastics to
> keep it down to a single class, wanting to avoid unnecessarily adding a
> bunch of classes to the system. Also wanted to impress Juan, who I expected
> would be hesitant to add a bunch of complicated machinery to Cuis.
>
> Looking back on the code something like a decade later, I think I was a
> little gung-ho with the one-class thing. Adding a single class
> UniformColorTheme (or maybe we can think of a better name like
> BoringColorTheme) seems to obviate #useUniformColors completely, and thus
> eliminate 13 send sites (which, I might add, all have a code duplication
> smell.) Each of these send sites involves branch logic that would go away,
> 3 or 4 lines of code each. I haven’t investigated this last one yet, but
> #defaultWindowColor might also go away, yielding a bit more in the way of
> complexity savings.
>
> Here’s a typical send site of Theme>>useUniformColors.
>
> Theme>>versionsBrowser
> ^ self useUniformColors
> ifTrue: [ self defaultWindowColor ]
> ifFalse: [ `(Color r: 0.869 g: 0.753 b: 1.0) duller` ]
>
> I’m not sure if I wrote the above code or not.
>
> It’s always ^self useUniformColors ifTrue:[] ifFalse[]. All 13 sites.
>
> Between one and two methods would go away along with an additional 39+
> lines of code. In exchange, we get a new class (my vote is for BoringTheme)
> with as many as 13 one-line methods. A cross-cutting concern goes away. We
> lose 5 LoC to a class definition, if we want to even count that, still a
> 21+ line win. I haven’t looked at the extra themes, but I’ll update them if
> I make the change, and I expect code savings there as well. More
> importantly, it will be more logical, more object oriented, less tricky.
> Right now to implement a theme, you need to know about this odd convention
> in these 13 methods, and it shouldn’t be that way. I went with methods
> instead of sticking instances of Color in ivars so we could experiment with
> functional expressions that yield arbitrary UI objects (originally there
> was going to be a lot more to this than colors, but life has a way of
> interrupting grand schemes.) Here though we’re throwing in logic to pick
> between two colors, which smells like the opposite idea.
>
> Okay that’s the gist of it. Only performance impact should be negligible
> but positive (sends and branch logic that don’t need to happen anymore.)
> Theme-Core isn’t at all a crowded category so scrolling and cognitive load
> shouldn’t be an issue with adding a class here. I can probably absorb what
> I think used to be called GrayTheme out of the extra themes package into
> OfficeDull Win95KnockOff or whatever we decide to call it, so any methods
> there go away from the extra themes package.
>
> Is everyone okay with me doing this?
>
> —Casey
> --
> Cuis-dev mailing list
> Cuis-dev at lists.cuis.st
> https://lists.cuis.st/mailman/listinfo/cuis-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20200525/3f4dceb9/attachment.htm>