Sharky hat mich gebeten mal zu erklaeren was ich mit Bugs und Unschoenheiten meine.
Ich verbreite mich deshalb mal zu verschiedenen Aspekten der Sache.
Zuerst der Code Style, denn da habe ich praktisch die gesamte JVCL ueberarbeitet und deshalb aergert mich das zuerst
Das sollte man durchaus ernst nehmen, da ich festgestellt habe das an besonders schlampigen Stellen auch meist die Fehler sitzen.
Delphi-Quellcode:
unit ShBlinkLabel;
interface
uses
StdCtrls, ExtCtrls, Classes;
// bis hier ist es sehr schoen
type
TShBlinkLabel =
class(TLabel)
private
// die offizielle Style-Regel ist das alle Variablennamen gross beginnen
fTimer: TTimer;
// Typen immer mit dem case den Delphi nennt, also hier "Integer"
// Ausnahme ist string, das zwar als String gemeldet wird aber ein Keyword ist und alle Keywords immer klein
fIntervall: integer;
fAktiv: boolean;
protected
// OnTimer ist ein sehr schlechter Name, da er von Delphi vergeben wird.
// Methodennamen sollten eine Taetigkeit benennen
procedure OnTimer(Sender: TObject);
// die offizielle Style-Regel ist das alle Variablennamen gross beginnen, auch Parameter
procedure SetAktiv(aValue: boolean);
procedure SetIntervall(aValue: integer);
public
constructor Create(AOwner: TComponent);
override;
destructor Destroy;
override;
// die Properties sollte man alphabetisch sortieren, damit es mit dem Objektinspector uebereinstimmt
// Intervall ist ein schlechter Name, da es einen Sprachmix einfuehrt
// Interval ist korrekt
// Direktiven wie Keywords klein, daher read und write
property Intervall: integer
Read fIntervall
Write SetIntervall;
// entsprechend Active
property Aktiv: boolean
Read fAktiv
Write SetAktiv;
// keine leeren Abschnitte
published
// keine Leerzeilen wo sie unnoetig sind
end;
implementation
{ TShBlinkLabel }
// die Parameter sollten immer wiederholt werden, damit man nicht so weit suchen muss
constructor TShBlinkLabel.Create;
begin
// alle inherited Aufrufe sollten explizit sein, da man dann immer weiss was aufgerufen wird
// Ausnahme sind message Methoden bei denen das nicht geht
inherited;
fAktiv := False;
// es heisst Self
fTimer := TTimer.Create(self);
fTimer.OnTimer := OnTimer;
fTimer.Interval := 500;
fTimer.Enabled := fAktiv;
end;
destructor TShBlinkLabel.Destroy;
begin
// siehe Konstruktor
fTimer.Free;
// siehe Konstruktor
inherited;
end;
procedure TShBlinkLabel.OnTimer(Sender: TObject);
begin
// harmlos, aber gelegentlich aergerlich
// immer ein Semikolon setzen, damit man bei Ergaenzungen keinen Syntaxfehler bekommt
// keine ueberfluessigen Klammern. Die stoeren nur beim Lesen
Visible :=
not (Visible)
end;
procedure TShBlinkLabel.SetAktiv(aValue: boolean);
begin
fAktiv := aValue;
fTimer.Enabled := aValue;
end;
procedure TShBlinkLabel.SetIntervall(aValue: integer);
begin
fTimer.Enabled := False;
fTimer.Interval := aValue;
fTimer.Enabled := True;
end;
end.
In der zweiten Runde kommen wir zu den Fehlern und und Unschoenheiten.
Delphi-Quellcode:
unit ShBlinkLabel;
interface
uses
StdCtrls, ExtCtrls, Classes;
type
TShBlinkLabel =
class(TLabel)
private
FTimer: TTimer;
FInterval: Integer;
FActive: Boolean;
protected
// in protected sind diese Methoden unsinnig, da sie nicht virtuell sind
procedure InternalTimerEvent(Sender: TObject);
procedure SetActive(AValue: Boolean);
procedure SetInterval(AValue: Integer);
public
constructor Create(AOwner: TComponent);
override;
destructor Destroy;
override;
// Properties muessen published sein und sollten so moeglich einen Default-Wert haben
property Active: Boolean
read FActive
write SetActive;
property Interval: Integer
read FInterval
write SetInterval;
end;
implementation
{ TShBlinkLabel }
constructor TShBlinkLabel.Create(AOwner: TComponent);
begin
inherited Create(AOwner);
FActive := False;
// hier ensteht eine Diskrepanz zum Destruktor
// soll die Komponente den Timer selbst abraeumen, dann braucht es im Destruktor kein Free
// anders herum sollte hier dann nil statt Self stehen, damit das Free im Destruktor effizienter ist
FTimer := TTimer.Create(Self);
FTimer.OnTimer := InternalTimerEvent;
// FInterval wird nicht gesetzt
FTimer.Interval := 500;
FTimer.Enabled := FActive;
end;
destructor TShBlinkLabel.Destroy;
begin
FTimer.Free;
inherited Destroy;
end;
procedure TShBlinkLabel.InternalTimerEvent(Sender: TObject);
begin
// Visible zu manipulieren ist voellig falsch
// es soll doch nur das Rendering der Komponente umgeschaltet werden, nicht eine Property
Visible :=
not Visible;
end;
procedure TShBlinkLabel.SetActive(AValue: Boolean);
begin
// prinzipiell sollten hier unnoetige Zuweisungen, die nichts veraendern, vermieden werden,
// aber diese Optimierung erfolgt ja schon im Timer fuer Enabled
FActive := AValue;
FTimer.Enabled := AValue;
end;
procedure TShBlinkLabel.SetInterval(AValue: Integer);
begin
// FInterval wird schon wieder nicht gesetzt
// Man sollte sich darauf verlassen das die Timerkomponente die Zuweisung an
// Interval schon selbst korrekt durchfuehrt, also ist die Manipulation von FTimer.Enabled unnoetig
FTimer.Enabled := False;
FTimer.Interval := AValue;
FTimer.Enabled := True;
end;
end.
Jetzt mit allem was dazugehoert.
Delphi-Quellcode:
unit ShBlinkLabel;
interface
uses
StdCtrls, ExtCtrls, Classes;
type
TShBlinkLabel =
class(TLabel)
private
FTimer: TTimer;
FInterval: Integer;
FActive: Boolean;
// hier halten wir uns den Darstellungszustand
FVisibleState: Boolean;
procedure InternalTimerEvent(Sender: TObject);
procedure SetActive(AValue: Boolean);
procedure SetInterval(AValue: Integer);
protected
// das brauchen wir zum richtigen Zeichnen
procedure Paint;
override;
public
constructor Create(AOwner: TComponent);
override;
destructor Destroy;
override;
published
property Active: Boolean
read FActive
write SetActive
default False;
property Interval: Integer
read FInterval
write SetInterval
default 500;
end;
implementation
{ TShBlinkLabel }
constructor TShBlinkLabel.Create(AOwner: TComponent);
begin
inherited Create(AOwner);
FActive := False;
FInterval := 500;
FTimer := TTimer.Create(
nil);
FTimer.OnTimer := InternalTimerEvent;
FTimer.Interval := FInterval;
FTimer.Enabled := FActive;
FVisibleState := True;
end;
destructor TShBlinkLabel.Destroy;
begin
FTimer.Free;
inherited Destroy;
end;
procedure TShBlinkLabel.InternalTimerEvent(Sender: TObject);
begin
// im Timer wird nur der Darstellungszustand geaendert
FVisibleState :=
not FVisibleState;
// und wir melden uns zum Neuzeichnen an
// Niemals sich selbst neuzeichnen, denn das ist ineffizient und hat auch noch andere Nebenwirkungen!
Invalidate;
end;
procedure TShBlinkLabel.SetActive(AValue: Boolean);
begin
FActive := AValue;
FTimer.Enabled := AValue;
// Sicherstellen das wir sichtbar enden
// mit Optimierung gegen unnoetiges Neuzeichnen
if not AValue
and not FVisibleState
then
begin
FVisibleState := True;
Invalidate;
end;
end;
procedure TShBlinkLabel.SetInterval(AValue: Integer);
begin
FInterval := AValue;
FTimer.Interval := AValue;
end;
procedure TShBlinkLabel.Paint;
begin
if FVisibleState
then
// wenn der Label sichtbar ist, dann lassen wir ihn von TLabel.Paint malen
inherited Paint
else
// Style-Anmerkung es gibt hier keine unnoetigen begin end Klammern
// Hier habe ich bei TCustomLabel.Paint nachgeschaut
// Der Label muss seinen Hintergrund malen, so er nicht transparent ist
with Canvas
do
if not Transparent
then
begin
Brush.Color := Self.Color;
Brush.Style := bsSolid;
FillRect(ClientRect);
end;
end;
end.
So dieser Blink-Label sollte deutlich lesbarer sein und korrekt funktionieren.
Ob er auch im Entwurfsmodus blinken sollte sei dahingestellt.
Dieser Code ist ungetestet, aber ich habe Vertrauen in ihn.