Kill KillWindow
Needs ReviewPublic

Authored by zzag on Wed, Feb 6, 8:00 PM.

Details

Reviewers
None
Group Reviewers
KWin
Summary

We don't need a class just for a single method.

Test Plan

"Kill window" functionality still works as before.

Diff Detail

Repository
R108 KWin
Branch
kill-kill-window
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7956
Build 7974: arc lint + arc unit
zzag created this revision.Wed, Feb 6, 8:00 PM
Restricted Application added a project: KWin. · View Herald TranscriptWed, Feb 6, 8:00 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Wed, Feb 6, 8:00 PM
zzag updated this revision to Diff 51054.Wed, Feb 6, 8:02 PM

Make diff a little bit nicer

zzag edited the summary of this revision. (Show Details)Wed, Feb 6, 8:03 PM
zzag edited the summary of this revision. (Show Details)

One of the long time problem of KWin was that we had too many things in Workspace. Workspace just does too much - it's a god object.

Given that I'm not totally thrilled of moving more code into it which was nicely separated in a dedicated class. To me the better question would be whether we could move the remaining bits which were kept in workspace into the KillWindow class. E.g. remove the slot in Workspace and direct all usages into KillWindow.

zzag added a comment.Sun, Feb 10, 5:55 PM

Workspace just does too much - it's a god object.

Agreed.

Given that I'm not totally thrilled of moving more code into it which was nicely separated in a dedicated class.

To be honest, so am I, but I also think that having a dedicated class for a single method is wrong, there is no cohesion. The right way would be to go with an utility function.

To me the better question would be whether we could move the remaining bits which were kept in workspace into the KillWindow class. E.g. remove the slot in Workspace and direct all usages into KillWindow.

Yeah, I was also thinking about that. I came to conclusion that moving KillWindow::start to Workspace::slotKillWindow would be more simpler, though I won't insist on that and I'm okay to abandon this revision.