Checkbox hell :)

Started by
6 comments, last by Maega 21 years, 5 months ago
Help.. I have 8 check boxes. If a checkbox is checked, the variable associated with that box is removed from comparison. I started doing this.. if (bStrength && bIntel && bWis && bWill && bCon && bAgil && bChari && bLuck) { if(Strength == WStrength && Intel == WIntel && Wis == WWis && Will == WWil && Con == WCon && Agil == WAgil && Luck1 == WLuck) { checkvariable = FALSE; } else { Click(); } } if(!bStrength && bIntel && bWis && bWill && bCon && bAgil && bChari && bLuck) { if(Intel == WIntel && Wis == WWis && Will == WWil && Con == WCon && Agil == WAgil && Chari == WChari && Luck1 == WLuck) { checkvariable = FALSE; } else { Click(); } } if(!bStrength && !bIntel && bWis && bWill && bCon && bAgil && bChari && bLuck) { if(Wis == WWis && Will == WWil && Con == WCon && Agil == WAgil && Chari == WChari && Luck1 == WLuck) { checkvariable = FALSE; } else { Click(); } } etc.. if I keep going with that.. I''ll come out with 64 ifs.. thats alot to type.. any suggestions on how to cut down the code? Maega
Advertisement
Put the different stats in an array or vector, indexed by enums. Then use for-loops (or, in the case of a vector, algorithms).

Don''t listen to me. I''ve had too much coffee.
Could you show an example of that please?

Ive never done anything like that before

To further explain my problem.. I'll just post my code so far hehe


  #include <windows.h>#include <stdio.h>#include "resource.h"BOOL CALLBACK Roller(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam);BOOL Read(DWORD address, BYTE* buffer);BOOL Check();void ClickButton();BYTE Strength = 0;BYTE Intel = 0;BYTE Wis = 0;BYTE Will = 0;BYTE Con = 0;BYTE Agil = 0;BYTE Chari = 0;BYTE Luck = 0;BYTE WStrength = 0;BYTE WIntel = 0;BYTE WWis = 0;BYTE WWill = 0;BYTE WCon = 0;BYTE WAgil = 0;BYTE WChari = 0;BYTE WLuck = 0;UINT ChStrength;UINT ChIntel;UINT ChWis;UINT ChWill;UINT ChCon;UINT ChAgil;UINT ChChar;UINT ChLuck;BOOL bStrength = TRUE;BOOL bIntel = TRUE;BOOL bWis = TRUE;BOOL bWill = TRUE;BOOL bCon = TRUE;BOOL bAgil = TRUE;BOOL bChari = TRUE;BOOL bLuck = TRUE;BOOL bWStrength = FALSE;BOOL bWIntel = FALSE;BOOL bWWis = FALSE;BOOL bWWill = FALSE;BOOL bWCon = FALSE;BOOL bWAgil = FALSE;BOOL bWChari = FALSE;BOOL bWLuck = FALSE;const DWORD Constitution = 0x00CFF38A;const DWORD Luck1 = 0x00CFF38E;const DWORD Strength1 = 0x00CFF385;const DWORD Intelligence = 0x00CFF387;const DWORD Wisdom = 0x00CFF388;const DWORD Willpower = 0x00CFF389;const DWORD Agility = 0x00CFF38B;const DWORD Charisma = 0x00CFF38D;HWND ghwnd;DWORD pid;HANDLE phandle;POINT screenpoint;BOOL checkvariable;int WINAPI WinMain(IN HINSTANCE hInstance, IN HINSTANCE hPrevInstance, IN LPSTR lpCmdLine, IN int nShowCmd){	HWND Dialog = CreateDialog(hInstance, MAKEINTRESOURCE(ROLLER), NULL, (DLGPROC)Roller);	ShowWindow(Dialog, SW_SHOW);	MSG msg;	int status;	while ((status = GetMessage (&msg, 0, 0, 0)) != 0)	{		if (status == -1)			return -1;    		if (!IsDialogMessage (Dialog, &msg)) 		{			TranslateMessage ( &msg );			DispatchMessage ( &msg );     		}	}	return msg.wParam; }BOOL CALLBACK Roller(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam){	switch (msg)	{	case WM_INITDIALOG:		ghwnd = FindWindow(NULL, "Kingdom of Drakkar Character Creation Version 2.0");		GetWindowThreadProcessId(ghwnd, &pid);		phandle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid);		SetTimer(hwnd, 1001, 100, NULL);		SetTimer(hwnd, 1002, 100, NULL);		SetTimer(hwnd, 1003, 100, NULL);		return TRUE;	case WM_COMMAND:		switch(LOWORD(wparam))		{			case IDINSTRUCTION:			return TRUE;		}		return TRUE;	case WM_TIMER:		if(wparam == 1001)		{			if(GetAsyncKeyState(VK_F2))			{					WStrength = GetDlgItemInt(hwnd, IDEStrength, NULL, NULL);				WIntel = GetDlgItemInt(hwnd, IDEIntelligence, NULL, NULL);				WWis = GetDlgItemInt(hwnd, IDEWisdom, NULL, NULL);				WWill = GetDlgItemInt(hwnd, IDEWillpower, NULL, NULL);				WCon = GetDlgItemInt(hwnd, IDEConstitution, NULL, NULL);				WAgil = GetDlgItemInt(hwnd, IDEAgility, NULL, NULL);				WChari = GetDlgItemInt(hwnd, IDECharisma, NULL, NULL);				WLuck = GetDlgItemInt(hwnd, IDELuck, NULL, NULL);				ChStrength = IsDlgButtonChecked(hwnd, IDCStrength);				ChIntel = IsDlgButtonChecked(hwnd, IDCIntel);				ChWis = IsDlgButtonChecked(hwnd, IDCWis);				ChWill = IsDlgButtonChecked(hwnd, IDCWill);				ChCon = IsDlgButtonChecked(hwnd, IDCCon);				ChAgil = IsDlgButtonChecked(hwnd, IDCAgil);				ChLuck = IsDlgButtonChecked(hwnd, IDCLuck);								if(ChStrength == BST_CHECKED)					bStrength = FALSE;				if(ChIntel == BST_CHECKED)					bIntel = FALSE;				if (ChWis == BST_CHECKED)					bWis = FALSE;				if(ChWill == BST_CHECKED)					bWill = FALSE;				if(ChCon == BST_CHECKED)					bCon = FALSE;				if(ChAgil == BST_CHECKED)					bAgil = FALSE;				if(ChChar == BST_CHECKED)					bChari = FALSE;				if(ChLuck == BST_CHECKED)					bLuck = FALSE;				checkvariable = TRUE;				Check();			}		}		if (wparam == 1002)		{			if (GetAsyncKeyState(VK_F3))			{				GetCursorPos(&screenpoint);				ScreenToClient(ghwnd, &screenpoint); 			}		}		if(wparam == 1003)		{			if (GetAsyncKeyState(VK_ESCAPE))			{				checkvariable = FALSE;			}		}		return TRUE;	case WM_CLOSE:		KillTimer(hwnd, 1001);		KillTimer(hwnd, 1002);		KillTimer(hwnd, 1003);		PostQuitMessage(0);		return TRUE;	}	return FALSE;}BOOL Read(DWORD address, BYTE* buffer){	ReadProcessMemory(phandle, (void *)address, buffer, 1, NULL);	return TRUE;}BOOL Check(){	while (checkvariable)	{		bWStrength = FALSE;		bWIntel = FALSE;		bWWis = FALSE;		bWWill = FALSE;		bWCon = FALSE;		bWAgil = FALSE;		bWChari = FALSE;		bWLuck = FALSE;				Sleep(2000);		Read(Strength1, &Strength);		Read(Intelligence, &Intel);		Read(Wisdom, &Wis);		Read(Willpower, &Will);		Read(Constitution, &Con);		Read(Agility, &Agil);		Read(Charisma, &Chari);		Read(Luck1, &Luck);		if (bStrength && Strength == WStrength)			bWStrength = TRUE;		if (bIntel && Intel == WIntel)			bWIntel = TRUE;		if (bWis && Wis == WWis)			bWWis = TRUE;		if (bWill && Will == WWill)			bWWill = TRUE;		if (bCon && Con == WCon)			bWCon = TRUE;		if (bAgil && Agil == WAgil)			bWAgil = TRUE;		if (bChari && Chari == WChari)			bWChari = TRUE;		if (bLuck && Luck == WLuck)			bWLuck = TRUE;				if (bWStrength && bWIntel && bWWis && bWWill && bWCon && bWAgil && bWChari && bWLuck)			checkvariable = FALSE;		else			ClickButton();	return TRUE;}	return TRUE;}void ClickButton(){	SendMessage(ghwnd, WM_LBUTTONDOWN, MK_LBUTTON, MAKELONG(screenpoint.x, screenpoint.y));	SendMessage(ghwnd, WM_LBUTTONUP, MK_LBUTTON, MAKELONG(screenpoint.x, screenpoint.y));}   


Here I have it so if checks to see if the variables match, so it sets a boolean saying it's true.. and then at the bottom, I check to see if they are all true..

But I want to be able to ignore certain stats from the comparison (you'll see that from the IsDlgButtonChecked) by checking the box. I am still unable to do it without writing the large amount of ifs.





[edited by - Maega on November 8, 2002 3:29:45 PM]

[edited by - Maega on November 8, 2002 3:31:10 PM]
Well, I'm not completely sure I comprehend what you're trying here... but...

if ((bStrength && (Strength != WStrength)) || (bIntel && (Intel != WIntel)) || (bWis && (Wis != WWis)) || (bWill && (Will != WWill)) || (bCon && (Con != WCon)) || (bAgil && (Agil != WAgil)) || (bChari && (Chari != WChari)) || (bLuck && (Luck != WLuck)))
Click();
else
checkvariable = false;

That look about right?

Basically it checks for each check box if the checkbox is checked AND the current stat value != the stored stat value, and if *any checkbox* is checked and the current stat value aren't equal to the stored, then it will hit the "click", otherwise it will set checkvariable to false.

Sorry if I got any of your variables wrong, I didn't verbatim-copy them all.

-fel



[edited by - felisandria on November 8, 2002 5:03:56 PM]
~ The opinions stated by this individual are the opinions of this individual and not the opinions of her company, any organization she might be part of, her parrot, or anyone else. ~


I thought it would be easier if I gave a screenshot .


As it says in the screenshot, if the checkbox is checked, the variable associated with it is not used in the comparison. So if ignore next to Strength was checked Strength/WStrength wouldn't be used in the final comparison checking to see if they all match.

It can be any combination of check boxes. They aren't grouped together in any way.

What Im asking is if there is a way to cut down the amount of ifs, because if I have to check every possible combination, it comes out to ALOT of ifs.


[edited by - Maega on November 8, 2002 7:53:21 PM]
Okay, then take my big if statement up there, and put a ! in front of all the checkbox variables.

-fel
~ The opinions stated by this individual are the opinions of this individual and not the opinions of her company, any organization she might be part of, her parrot, or anyone else. ~
quote:Original post by Maega
any suggestions on how to cut down the code?

You also have a problem that the code is hard to understand (which is partly related to the code being long). If I were writing this, I''d start by building some layers of abstraction to enhance the clarity of the code.

The first thing I''d do is to write some code which calls your big comparison with various values, ensuring that the code does what is expected. Then, I''d pull the comparison out into a function, something like this:

bool checkVariable(int lhs, int rhs, bool flag =true){	if(flag)		return true;	else		return lhs == rhs;} 


And I''d get the result from all comparisons using something like this:

checkvariable = !(checkVariable(Strength, WStrength, bStrength) 		&& checkVariable(Intel, WIntel, bIntel)		/* and so on */); 


Additionally, I would choose better names for the variables, but I don''t know enough about what you are doing to make any suggestions there. If you find good names for the function and the variables, the code will be a lot clearer.

When making these changes, I''d keep compiling and running the tests to ensure that I hadn''t broken the code, modifying the tests where necessary.

The next thing I''d do would be to look for further improvements. Since you have associated data and behaviour, I''d suspect a class might be hiding amongst the code, and since you seem to be comparing two instances of the class, I''d guess you might want to have something like this...

class Character{public:	Character(int strength, int intelligence, int wisdom, int willpower)		: strength_(strength),		intelligence_(intelligence),		wisdom_(wisdom),		willpower_(willpower)		// ...	{	}	bool operator==(const Character& rhs) const	{		// ...	}	private:	const int strength_;	const int intelligence_;	const int wisdom_;	const int willpower_;	// ...}; 


The thing I''m unclear of is exactly what part the "ignore" flags play in your code, so I''m unsure whether they belong in the class or not - maybe as static member data - and what sort of effect they would have on the operator==, or whether operator== is really the correct function to do the comparison. However, I''d expect the code to end up as something like this:

Character bloke(Strength, Intelligence, Wisdom, Willpower);const Character baseline(WStrength, WIntelligence, WWisdom, WWillpower);// do something to set the "mask" for comparisonif(bloke == baseline)  checkvariable = false;else  Click(); 


I''d possibly go further than that, and extract the above if..else into another function, but I *really* don''t know enough about your program to be sure about that. It''s also possible that there would be reworking elsewhere in the program which would allow you to change the entire way this particular bit of code needs to be written.
Im at school so Im not logged in.

I fixed the problem with Sneftel''s suggestion. I made an array of bytes for my values being read, and a set for my wanted values. I also made an array of bools for the check boxes.

I then just used a for loop to check .

Worked fine

Thanks everyone

Maega

This topic is closed to new replies.

Advertisement