<div><div dir="auto">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:</div></div><div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 25 May 2020 at 5:33 PM, Casey Ransberger via Cuis-dev <<a href="mailto:cuis-dev@lists.cuis.st">cuis-dev@lists.cuis.st</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;padding-left:1ex;border-left-color:rgb(204,204,204)"><div style="word-wrap:break-word;line-break:after-white-space">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.<div><br></div><div>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.</div><div><br></div><div><font style="color:rgb(0,0,0)"><span>Here’s a typical send site of Theme>>useUniformColors.</span></font><div style="color:rgb(0,0,0)"><br></div><div style="color:rgb(0,0,0)"><div>Theme>>versionsBrowser</div><div><span style="white-space:pre-wrap"> </span>^ self useUniformColors</div><div><span style="white-space:pre-wrap">          </span>ifTrue: [ self defaultWindowColor ]</div><div><span style="white-space:pre-wrap">              </span>ifFalse: [ `(Color r: 0.869 g: 0.753 b: 1.0) duller` ]</div></div><div style="color:rgb(0,0,0)"><br></div><div style="color:rgb(0,0,0)">I’m not sure if I wrote the above code or not.</div></div><div><br></div><div>It’s always ^self <span style="color:rgb(0,0,0)">useUniformColors ifTrue:[] ifFalse[]. All 13 sites.</span></div><div><br></div><div>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. <span style="color:rgb(0,0,0)">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.</span></div><div><br></div><div>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.</div><div><br></div><div>Is everyone okay with me doing this?</div></div><div style="word-wrap:break-word;line-break:after-white-space"><div><br></div><div>—Casey</div></div>-- <br>
Cuis-dev mailing list<br>
<a href="mailto:Cuis-dev@lists.cuis.st" target="_blank">Cuis-dev@lists.cuis.st</a><br>
<a href="https://lists.cuis.st/mailman/listinfo/cuis-dev" rel="noreferrer" target="_blank">https://lists.cuis.st/mailman/listinfo/cuis-dev</a><br>
</blockquote></div></div>