Verbesserungsvorschläge für Code

[It0101]

@Zhavok sagte in Verbesserungsvorschläge für Code:

@wob Der Integer je nach System meist 4 Byte. Bei Microcontrollern auch mal 2 Byte. Grundsätzlich >= short und <= long. Die Adresse ist wahrscheinlich wie ein Integer von der Größe her. So viele Bytes dürfte er nicht brauchen um die Adresse zu speichern. Bei Klassen ist das komplexer. Sie benötigen mehr Speicher. Richtig? Und wenn ja, dann kann ich verstehen warum es keinen Sinn macht und werde es ändern, aber direkt negative Auswirkungen dürfte es nicht haben oder?

https://stackoverflow.com/questions/3009543/passing-integers-as-constant-references-versus-copying
Depending on the underlying instruction set, an integer parameter can be passed as register or on the stack. Register is definitely faster than memory access, which would always be required in case of const refs (considering early cache-less architectures)

[Swordfish]

Alle Verallgemeinerungen sind falsch. Grundsätzlich.

Da jede Referenz auch eine Indirektion bedingt (nicht laut Standart aber de facto) ist zwischen Kosten der Dereferenzierung (Anzahl?) und Kosten der Kopie (= bessere cache-locality) abzuwägen. Ich kann mir aber auch compiler vorstellen, die bei der Übergabe einer const & garnichts übergeben, sondern auf dem Original arbeiten.

[Zhavok]

Ok super. Das mit den Integer-Referenzen wusste ich noch nicht. Schön das zu wissen, da wäre ich nicht so schnell drauf gekommen. Ich merke, dass meine Fehler der Faulheit geschuldet sind. Einiges was ihr geschrieben habt, hatte ich schon im Kopf gehabt und mit einem "Ach naja, ist ja nicht ganz so wichtig" abgehackt. Das betrifft die Prüfung des JSONs und die Möglichkeit die Koordinaten unter einem Dach zu haben. Ich muss mich wohl zwingen auch die unschönen Vorgänge nicht zu vernachlässigen. Und da gehört die Überprüfung von Eingaben etc. bei mir dazu

Ich hoffe jetzt alles beachtet zu haben was ihr geschrieben habt. Bei der Prüfung der Position ist mir nichts eingefallen, da jetzt negative Werte auch noch erlaubt sind und ich nicht weiß, bis wohin es sinnig ist die Größe zu begrenzen.

#include <iostream>
#include <string>
#include <fstream>
#include <Windows.h>
#include <thread>
#include <nlohmann/json.hpp>



struct Point {
	int xPos;
	int yPos;
};



class KeyHandler {


public:
	KeyHandler(const std::string &windowTitle, const std::string &key, const Point position)
		: windowTitle(windowTitle), key(key), position(position) {

	}

	bool keyPressed() const {
		if (GetAsyncKeyState(key[0]) && GetAsyncKeyState(VK_LCONTROL))
			return true;
		else
			return false;
	}



	void doClick() const {

		//Fenster suchen
		HWND wndH = FindWindow(NULL, windowTitle.c_str());

		if (wndH == 0) {
			std::cout << "Fenster nicht gefunden!" << std::endl;
			return;
		}

		//Fenster in den Vordergrund bringen
		SetForegroundWindow(wndH);
		BringWindowToTop(wndH);

		//Maus positionieren
		SetCursorPos(position.xPos, position.yPos);


		//Mausklick simulieren
		INPUT Input = { 0 };

		Input.type = INPUT_MOUSE;
		Input.mi.dwFlags = MOUSEEVENTF_LEFTDOWN;
		::SendInput(1, &Input, sizeof(INPUT));

		::ZeroMemory(&Input, sizeof(INPUT));
		Input.type = INPUT_MOUSE;
		Input.mi.dwFlags = MOUSEEVENTF_LEFTUP;
		::SendInput(1, &Input, sizeof(INPUT));
	}
	

private:
	const std::string windowTitle;
	const std::string key;
	const Point position;

};



std::vector<KeyHandler> readConfig() { //config lesen und parsen

	std::ifstream input("config.cfg");
	std::string jsonString((std::istreambuf_iterator<char>(input)),(std::istreambuf_iterator<char>()));
	nlohmann::json config = nlohmann::json::parse(jsonString);
	std::vector<KeyHandler> retVec;
	retVec.reserve(config["shortcuts"].size());

	for (unsigned int i = 0; i < config["shortcuts"].size(); i++) {
		const auto &myref = config["shortcuts"][i];

		if (myref[0].dump().length() == 2) { //dump() besteht leer aus einem "" string
			std::cout << "Fehler: Keine Fenstertitel gesetzt!" << std::endl;
			break;
		}
		if (myref[1].dump().length() == 2) {
			std::cout << "Fehler: Keine Taste für Kombination nicht gesetzt!" << std::endl;
			break;
		}

		Point position = { myref[2], myref[3] };
		retVec.emplace_back(myref[0], myref[1], position);
	}


	return retVec;
}



int main() {

	std::vector<KeyHandler> keyHandler = readConfig();
	
	while (true) {
		for (const auto &keys : keyHandler) {
			if (keys.keyPressed())
				keys.doClick();
		}

		std::this_thread::sleep_for(std::chrono::milliseconds(100)); //delay damit nicht zuviele Klicks ausgeführt werden
	}
}

[It0101]

@Swordfish sagte in Verbesserungsvorschläge für Code:

Alle Verallgemeinerungen sind falsch. Grundsätzlich.

Da jede Referenz auch eine Indirektion bedingt (nicht laut Standart aber de facto) ist zwischen Kosten der Dereferenzierung (Anzahl?) und Kosten der Kopie (= bessere cache-locality) abzuwägen. Ich kann mir aber auch compiler vorstellen, die bei der Übergabe einer const & garnichts übergeben, sondern auf dem Original arbeiten.

Konsens ist aber dass es im Grunde egal ist. Vorteil von Call-By-Value ist die mögliche Arbeit mit Literalen und weniger Schreibarbeit

[wob]

@It0101 sagte in Verbesserungsvorschläge für Code:

da der Standard meines Wissens keine [POINT-Klasse] anbietet, müsste man ja selber eine bauen

Es wird hier Windows.h includet... Warum sollt man selber eine bauen? Hier wird SetCurPos benutzt. Das Gegenstück dazu, GetCursorPos, liefert sogar einen (Pointer auf) POINT zurück.

[It0101]

Ne Point-Klasse könntest du dir ruhig gönnen. Private Member, Konstruktor, Getter. Copy-Construktor, restliche Operatoren könnte man deleten.

[wob]

@It0101 sagte in Verbesserungsvorschläge für Code:

Private Member, Konstruktor, Getter. Copy-Construktor

Ich halte es da lieber einfach mit einer struct und 2 öffentlicher Member. Sehe da keinen Vorteil einer Klasse. Und warum nicht POINT nutzen? Wozu eine eigene Klasse?

[It0101]

@wob sagte in Verbesserungsvorschläge für Code:

@It0101 sagte in Verbesserungsvorschläge für Code:

Private Member, Konstruktor, Getter. Copy-Construktor

Ich halte es da lieber einfach mit einer struct und 2 öffentlicher Member. Sehe da keinen Vorteil einer Klasse. Und warum nicht POINT nutzen? Wozu eine eigene Klasse?

Das POINT ist doch keine Klasse, oder? Ich mag Klassen
Ich bin tatsächlich kein großer Freund der WINAPI. Zuviel C. Daher versuche ich so wenig wie möglich davon zu nutzen.

[Zhavok]

@wob ahhhhhh. So ist das also. Gut, dann baue ichs morgen nochmal um

@It0101 mal abseits davon, dass es ja wie @wob gesagt hat eine POINT Klasse gibt, wäre es nicht zuviel Overhead wegen den 3-4 mal so es vorkommt gleich ne Klasse mit Methoden etc. zu bauen? Deshalb hatte ich mich für ein struct entschieden.

[It0101]

@Zhavok Ist Geschmacksache. Ich denke in dem Fall ist beides gleichwertig.

Ich finde halt

POINT p;
p.x = 1;
p.y = 1;
functionCall( p );

nicht so schön wie

functionCall( MyPoint( 1, 1 ) );

[wob]

@It0101 Du kannst geschwungene Klammern benutzen.

[It0101]

@wob sagte in Verbesserungsvorschläge für Code:

@It0101 Du kannst geschwungene Klammern benutzen.

Ach ja stimmt... Egal!

[Swordfish]

@It0101 sagte in Verbesserungsvorschläge für Code:

@Zhavok Ist Geschmacksache. Ich denke in dem Fall ist beides gleichwertig.

Ich finde halt [..] nicht so schön wie [...]

ich wär' für 'ne Hall of Fame für soetwas ;p Schade, daß es TGGCs Helden nicht mehr gibt

[DocShoe]

@It0101
In dem Fall, dass POINT{ x, y } nicht funktioniert könnte man immer noch sowas wie

POINT make_point( LONG x, LONG y )
{
   POINT retval;
   retval.x = x;
   retval.y = y;
   return retval;
}

machen. Meiner Erfahrung nach fährt man oft besser mit den bestehenden Typen der API, die man benutzt, weil man sie doch häufiger braucht, als man denkt. Früher oder später muss man sie doch konvertieren, das nervt irgendwann

[It0101]

[DocShoe]

??

[It0101]

@DocShoe sagte in Verbesserungsvorschläge für Code:

??

Meine Denke ist halt: wenn man sich extra ne Funktion baut, die zwei ints in ein Point verfrachtet, kann man auch gleich ne Klasse mit nem Konstruktor bauen...

[wob]

@It0101 Dann denken wir anders. Ich sehe keinen einzigen Vorteil der eigenen Klasse außer deine Denke glücklich zu machen.

[It0101]

@wob sagte in Verbesserungsvorschläge für Code:

@It0101 Dann denken wir anders. Ich sehe keinen einzigen Vorteil der eigenen Klasse außer deine Denke glücklich zu machen.

Aber einen Vorteil des structs oder der vordefinierten POINT-Struktur konnte auch niemand nennen. Es ist halt einfach Geschmackssache

[Unterfliege]

Ein Problem bei zu vielen Point-Klassen, das ich mir gut vorstellen kann, sind Namenskonflikte (wenn keine namespaces genutzt werden).
Ich würde aber vermutlich tatsächlich selbst auch ein eigenes struct anlegen mit x, y als public member und ggf. einem Cast-Operator in POINT. Einen ctor kann man ja sonst auch schreiben. Nur getter/setter würde ich dabei definitiv weglassen, da es einfach überflüssig ist.
Die make_point-Funktion finde ich auch eine unnötig komplizierte Lösung. Dann lieber einfach eine Wrapper-Klasse oder so.