[PATCH] On Screen Display
Jakob Schroeter
js at camaya.de
Thu Jan 30 12:56:33 GMT 2003
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Thursday 30 January 2003 10:39, Simon Hausmann wrote:
> I think KOSD is a bad class name, because it lacks any
> descriptivity. Why not go for a verbose name, like KOnScreenDisplay
> or something?
Agreed, changed it to the verbose.
> + kosd->showOSD();
> Who deletes the kosd?
> [...]
> Same thing.... who's going to delete the kosd object here?
It is now deleteLater()'ed and a pointer is returned.
> +void KOSD::setOSDFont(QFont qfont)
> [...]
> +void KOSD::setOSDFont(const QString fontName, const int fontSize, const
> QFont::Weight fontWeight)
>
> Why this method in the first place? Convenience? IMHO it's not worth
> it. I think it's better to have only one method for setting the
> font, the one that QWidget provides. Gives a simpler (less
> cluttered) API. And it's not that creating a QFont object is
> terribly complex thing, especially given that most of the time one
> wants to use one of the font methods of KGlobalSettings anyway and
> adjust some properties on the returned font object, instead of doing
> QFont( "helvetica" ) or something in the application code, which is
> very bad.
I guess I was in the mood of creating a lot of new API. But I think you're
right. Removed.
> + enum osdPosition {
> [...]
> + };
removed in favour of Qt::AlignmentFlags as proposed by Aaron.
> + static void message( QWidget *parent, const QString &text, const int
> [...]
> I wonder if it's a good idea to provide such a function. One the
> first sight it seems like convenience, but I think it does not
> really help to keep code readable because of the huuuuge amount of
> parameters it takes.
I introduced this monster with the preview function of the kcm. I agree that
it's far easier to understand the customization of an own object than the
call to this function.
I've kicked it out because I don't feel like keeping track of additional
functionality within this, too.
> Why the OSD infix in the setters, btw? It seems redundant to me.
Well, IIRC, I introduced them with that setOSDFont() function... ;)
New patch will follow.
Jakob
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
iD8DBQE+OSEGRIytGOFpChERAjGKAJ9AI5604F+M2IoioAncN9gZKC9mHgCeOAso
9j5Fmbby1nqnI2GNueq3L6I=
=xXhF
-----END PGP SIGNATURE-----
More information about the kde-core-devel
mailing list