Stilistisch okay?

Hiho.

Hab ein Programm das verschiedene Methoden hat, die allesammt OnClick-Responses auf entsprechende MenuItems sind.

Zusätzlich habe ich Systemweite Hotkey's eingebaut (WIN+B als Beispiel)

Die die HotKey's alle die selbe Methode aufrufen (und nicht jeder HotKey eine eigene) rufe ich nach Überprüfung welcher HotKey gedrückt worden ist die OnClickMethode der MenuItems auf (Sender setze ich dann NULL).

Das funktioniert, keine Frage.
Aber ist das so "richtig", oder sollte man soetwas nicht machen?

Wenn es nicht okay ist, wie macht das dann am Geschicktesten?
Weil ich möchte nicht den jeweiligen Anweisungsblock mehrfach im Quelltextstehen haben. (Redundanz)

Und ich dachte mir, so spare ich mir auch noch weitere Methoden, die nur die Funktionalität hätten und die ich dann in den OnClick sowie in der GetHotKey Methoden aufrufen würde.

Wenn ihr versteht was ich meine

Ich freue mich auf eure Antworten

bye

PS: Ich wusste nicht genau wie ich den Thread nennen sollte.
Wenn einem Mod ein besserer einfällt bitte Ändern.

Acidmrp

warum sollte das nicht ok sein?
Es ist übersichtlich und funktioniert.

Einzig eine Änderung solltest du dir überlegen:

void __fastcall TForm1::cmdLoadFileClick(TObject *Sender)
{
    LoadFile();
}

void __fastcall TForm1::Shortkey()
{
    switch (Shortkeys) {
        case 'L':
            LoadFile();
            break;
        case 'W':
            // ...
            break;
        // ...
    }
}

void __fastcall TForm1::LoadFile()
{
    //...
}

das finde ich noch etwas übersichtlicher.

Also so habe ich es bisher...

void __fastcall TAmpHook::FormCreate(TObject *Sender)
{ 
[.....]
RegisterHotKey(Handle,HotKeyWinYAtom,MOD_WIN,static_cast<int>('Y'));

RegisterHotKey(Handle,HotKeyWinXAtom,MOD_WIN,static_cast<int>('X'));
[.....]
}

//Fehlerübrprüfung und Rest mal rausgenommen

Dadurch wird ja dann eine WM_HOTKEY Message gesendet.
Die fange ich ab/auf und bearbeite sie so:

void __fastcall TAmpHook::KeyHook(TMessage &Message)
{
[.....]
  if ( Message.WParam==HotKeyWinYAtom )
    PlaybackPreviousClick(NULL);
  if ( Message.WParam==HotKeyWinXAtom )
    PlaybackStartClick(NULL);
[.....]
}

PlaybackPreviousClick und PlaybackStartClick sind halt einfach die OnClick methoden...

void __fastcall TAmpHook::PlaybackPrevious(TObject *Sender)
{
[.....]
// Ganzer Funktionsblock für PlaybackPrevious
}
void __fastcall TAmpHook::PlaybackStart(TObject *Sender)

{
[.....]
// Ganzer Funktionsblock für PlaybackStart
}

Du hast ja noch ne extra Methode gemacht, die dann sowohl vom HotKey als auch vom OnClick aufgerufen wird.
Auch ne Möglichkeit, macht das Prgramm aber wieder ein Stück grösser.

Soll ein Tool werden, das dauerhaft im Windows mitlaufen kann, daher sollte es so klein wie Möglich sein. Deswegen dachte ich spare ich so was Platz

Aber wenn das so okay is, wie es is, dann lass ichs so
Is ja "mein" system, mit dem ich gut zurechtkomme

HEZ

Styl0r schrieb:

Du hast ja noch ne extra Methode gemacht, die dann sowohl vom HotKey als auch vom OnClick aufgerufen wird.
Auch ne Möglichkeit, macht das Prgramm aber wieder ein Stück grösser.

..., übersichtlicher, nachvollziehbarer und einfach sinnvoller. Willst du jetzt den Stil verbessern oder nicht?

Und viel größer wird das Programm dadurch auch nicht. Falls überhaupt.

Ja.. Doch klingt einleuchtend
Dann werd ichs umschreiben und den Rest des Programms gleich so schreiben

Alexander Kempf

Acidmrp schrieb:

warum sollte das nicht ok sein?

Da fallen vielen sicherlich viele Gründe ein.

Acidmrp schrieb:

Es ist übersichtlich

Meines Erachtens nicht. Es ist vielleicht weniger Code, aber für mein Verständis sollte eine Ereignisbehandlungsmethode nur von dem Objekt
gerufen werden, das das Ereignis auslöst. Ich habe mir auch angewöhnt, die Methodennamen umzubenennen, wenn eine Methode mehr als einem Objekt
zugeordnet wird. Am besten ist aber wahrscheinlich die zusätzliche Methode.
Weniger Zeilen kann man z.B. auch erreichen, indem man mehrere Befehle in eine Zeile schreibt - aber ist das übersichtlicher?

Acidmrp schrieb:

und funktioniert.

Das Argument ist für mich ein rotes Tuch. Das klingt wie: "Ich überhole immer in unübersichtlichen Kurven und es ist noch nie was passiert."
Es gibt viele Programme, die funktionieren, aber praktisch unwartbar sind.

Gruß,

Alexander

Alexander Kempf schrieb:

Da fallen vielen sicherlich viele Gründe ein.

Hättest du da welche?
Genau sowas möchte ich ja.

Alexander Kempf schrieb:

sollte eine Ereignisbehandlungsmethode nur von dem Objekt gerufen werden, das das Ereignis auslöst.

Genau wegen diesem Gedanken habe ich gefragt.

Alexander Kempf schrieb:

"Ich überhole immer in unübersichtlichen Kurven und es ist noch nie was passiert."

naja ob man den Vergleich so zählen kann..

bye

Acidmrp

@Alexander Kempf

ich glaube du hast mich etwas falsch verstanden. Es ist auf jeden Fall übersichtlicher
und besser wie wenn er den entsprechenden Code zweimal in das Programm einbaut.
Eine Möglichkeit wie er es voll ganz korrekt machen kann hab ich ja auch noch dazu
geschrieben.

Die Sache mit dem OnClick funktioniert doch auch immer auch wenn es noch nicht 100%
übersichtlich ist. Oder wann klappt das nicht? Mit dem blinden überholen in Kurfen
bei Glatteis und Nebel ist das IMO nicht zu vergleichen. Da gibt es Programmtechnisch
schlimmere Dinge die man machen kann.

Alexander Kempf

Styl0r schrieb:

Hättest du da welche?

Ein Negativbeispiel erlebe ich öfters in unserem Team. Ein paar Leute haben sich angewöhnt Ereignisbehandlungsmethoden auch
von außen aufzurufen. Es könnten also im ganzen Projekt (mit ~40 Formularen) irgendwo Aufrufe einer solchen Methode von außen
erfolgen. Natürlich wird das auch dadurch begünstigt, dass diese Methoden in dem __published-Abschnitt stehen; meiner Meinung
nach sollten die Methoden im private- oder protected-Abschnitt stehen.
Problematisch wird das beispielsweise dann, wenn man das übergebene Objekt (meist TObject*) in irgendeiner Form auswertet. Im
schlimmsten Fall verlässt man sich darauf, dass das Objekt natürlich dasselbe ist, das auch das Ereignis ausgelöst hat. Wenn
dann NULL übergeben wurde und man vergessen hat, genau das zu berücksichtigen, knallt's natürlich - meistens erst beim Kunden,
weil ja der Programmierer gar nicht auf die Idee gekommen ist, dass die Methode von einem anderen Formular aufgerufen wurde.
Entscheidet man sich im Nachhinein dafür dieselbe Ereignisbehandlungsmethode für mehrere Objekte zu verwenden und hat eine
Fallunterscheidung eingefügt, wird ein übersehener Aufruf mit NULL im besten Fall verpuffen. Es gibt keinen Fehler, aber halt
auch keine Funktion.

Wenn in einer Ereignisbehandlungsmethode so grundsätzliche Dinge gemacht werden, dass man diese Methode gerne an mehreren Stellen
aufrufen möchte, dann ist das ein starkes Anzeichen dafür, die Funktionalität in eine neue Methode zu packen, der man dann auch
einen aussagekräftigeren Namen als Button1Click() geben kann - und sollte.

Wenn man so will geht's um Zuständigkeiten und um bessere Lesbarkeit. Beides erhöht die zukünftige Wartbarkeit.
Und dann kommt natürlich gleich das Argument: "Ja, aber das ist doch nur so ein kleines Progrämmchen - das wird niiiiiiiie
erweitert werden."
Dazu kann ich bloß sagen:
1. Wer weiß?
2. Falls jemand die Deutsche Fußballnationalmannschaft kennt - da gibt es immer die Ausrede: "Ja, gut, das war halt wieder nur
ein Freundschaftsspiel. Bei der <beliebiges internationales Turnier> wird natürlich alles besser." (Das Ergebnis kennen wir)
Was ich damit sagen will: Man gewöhnt sich schlechten Stil an.

Styl0r schrieb:

Alexander Kempf schrieb:

"Ich überhole immer in unübersichtlichen Kurven und es ist noch nie was passiert."

naja ob man den Vergleich so zählen kann..

Wie das mit Vergleichen so ist: Ab und zu muss man ein wenig übertreiben. Natürlich wird niemand daran sterben, wenn man in einem
Programm mal eine Ereignisbehandlungsroutine aus dem Code aufruft - wenn man nicht gerade in der Medizintechnik arbeitet.
Was hier im schlimmsten Fall irgendwann stirbt, ist das Programm bzw. Projekt.

Acidmrp schrieb:

Es ist auf jeden Fall übersichtlicher
und besser wie wenn er den entsprechenden Code zweimal in das Programm einbaut.

Da gebe ich Dir allerdings recht - und damit kämpfe ich auch permanent

Acidmrp schrieb:

Eine Möglichkeit wie er es voll ganz korrekt machen kann hab ich ja auch noch dazu
geschrieben.

Das stimmt - habe ich im Eifer des Gefechts wohl übersehen. Ich bitte Dich um Entschuldigung.

Gruß,

Alexander