I would like your opinion on this code

closed account (o35DwA7f)
Hello everybody. This is a sudoku with 2d array. It prints it calculates and it almost solves itself. What do you think about it? What am i doing wrong? Is the structure of the program ok? Are there flaws in it? Please feel free to judge and point out whatever you think is out of order. I am a pretty down to earth person but when c++ has problems Stroustrup calls me. Feast your eyes on this code. As always you can make fun of me or the code freely. Thank you.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
#include <iostream>
#include <iomanip>
#include <ctime>
#include <cstdlib>
#include <thread>
using namespace std;
const int length{ 9 };
void showthesudoku(int array[][length]);
int theinterface(char move);
int randomnumber();
int randomrow();
int randomcol();
void thanksforplaying();
int thegame();
int humaninput(int array[][length]);
int machineinput(int array[][length]);
int doesitfit(int array[][length]);
int calculations(int array[][length]);

struct astruct
{
	int precious[length][length]{ 0 };
	int row{ 0 };
	int col{ 0 };
	int number{0};
	int dummy[length][length]{ 0 }; 
	char move;
	int additionrow{ 0 };
	int additioncol{ 0 };
	int addition3x3box{ 0 };
}sud; 

int main()
{
	theinterface(sud.move);
	thanksforplaying();
	return 0;
}

int theinterface(char move)
{
	sud.move = 'a';
	while (true)
	{
		cout << "press q to quit, or p to play\n";
		cin >> sud.move;
		if (sud.move == 'q')
		{
			cout << "i spend days making this, #@#??@#\n";
			break;
		}
		else if (sud.move != 'p' && sud.move != 'q')
		{
			cout << "use the keyboard, ok? Stump your foot once for yes and two for no\n";
		}
		else if (sud.move == 'p')
		{
			cout << "thank you and enjoy\n";
			thegame();
			break;
		}
	}
		return 0;
}

int thegame()
{
	sud.row = 0; //i put these here so they reset every time to 0
	sud.col = 0;
	sud.number = 0;
	sud.dummy[sud.row][sud.col] = 0;

	sud.move = 'a';// i put this so it has not a q value from the previous menu.
	while (1)
	{
		cout << "press c to choose numbers or m for the machine to do it for you or q to quit\n";
		cin >> sud.move;
		if (sud.move == 'c')
		{

			humaninput(sud.precious);
			doesitfit(sud.precious);
			if (sud.number != 0)
			{
				sud.precious[sud.row][sud.col] = sud.number;
			}
			showthesudoku(sud.precious);
		}
		else if (sud.move == 'm')
		{
			machineinput(sud.precious);
			doesitfit(sud.precious);
			if (sud.number != 0)
			{
				sud.precious[sud.row][sud.col] = sud.number;
			}
			showthesudoku(sud.precious);
		}
		else if (sud.move == 'q')
		{
			break;
		}
		
	}
	return 0;
}

int doesitfit(int array[][length])
{	//spent about a lot of hours here, for this thing
	//checks every row for the same number
		for (int j = 0, i = sud.row; j < length; j++)
		{											   //|not needed|    | this one | excludes the number we choose
			if (((sud.number == sud.precious[i][j]) && ((sud.row == i && sud.col != j) && sud.precious[i][j] != 0)))
			{
				
				cout << "not possible, you already have one in the same line\n";
				sud.row = 0;
				sud.col = 0;
				sud.number = 0;
				sud.dummy[sud.row][sud.col] = 0;
				
				break;// not needed
			}
			
		}

		//checks every column for the same number
		for (int i = 0, j = sud.col; i < length; i++)
		{
			if (((sud.number == sud.precious[i][j]) && ((sud.row != i && sud.col == j) && sud.precious[i][j] != 0)))
			{
				cout << "not possible, you have one in the same column\n";
				sud.row = 0;
				sud.col = 0;
				sud.number = 0;
				sud.dummy[sud.row][sud.col] = 0;
				
				break;
			}
		}

		//this one i hate. try to exclude the same number in a 3x3 box spend 6 or more hours here. stop laughing
		int resizedrow = (sud.row / 3) * 3;
		int resizedcolumn = (sud.col / 3) * 3;
		for (int i = resizedrow; i < (resizedrow + 3); i++)
		{
			for (int j = resizedcolumn; j < (resizedcolumn + 3); j++)
			{							
				//Always checks for 4 boxes. the diagonal positions if you choose middle box.A 2x2 if you choose corner box and so on
				if (((sud.number == sud.precious[i][j]) && ((sud.row != i && sud.col != j) && sud.precious[i][j] != 0)))
				{
					cout << "you cant, i wish you could\n";
					sud.row = 0;
					sud.col = 0;
					sud.number = 0;
					sud.dummy[sud.row][sud.col] = 0;
					i = 10;
					break;
				}

			}
		}

		calculations(sud.precious);
		if (sud.additionrow > 45)
		{
			cout << "you exceed the max sum allowed in this row\n";
			sud.row = 0;
			sud.col = 0;
			sud.number = 0;
			sud.dummy[sud.row][sud.col] = 0;
		}
		else if (sud.additioncol > 45)
		{
			cout << "you exceed the max sum allowed in this column\n";
			sud.row = 0;
			sud.col = 0;
			sud.number = 0;
			sud.dummy[sud.row][sud.col] = 0;
		}
		else if (sud.addition3x3box > 45)
		{
			cout << "you exceed the max sum allowed in this box\n";
			sud.row = 0;
			sud.col = 0;
			sud.number = 0;
			sud.dummy[sud.row][sud.col] = 0;
		}	
		//absolutely worth it
		return 0;
}

int calculations(int array[][length])
{
	sud.additionrow = 0;//calculates the sum of all the numbers in the row the user chose, from his row input
	for (int i = sud.row, j = 0; j < length; j++)
	{
		sud.additionrow += sud.precious[i][j];
	}
	sud.additionrow+=sud.dummy[sud.row][sud.col];

	sud.additioncol = 0;//calculates the sum of all the numbers in the column the user chose, from his column selection
	for (int i = 0, j = sud.col; i < length; i++)
	{
		sud.additioncol += sud.precious[i][j];
	}
	sud.additioncol += sud.dummy[sud.row][sud.col];

	sud.addition3x3box = 0;//calculates the sum of all the numbers in a 3x3square with magic. learned that here, ty.
	int resizedrow = (sud.row / 3) * 3;     //3 possible outcomes each to get the 3x3box.(0,3,6)^2=1 to 9 possible 3x3box 
	int resizedcolumn = (sud.col / 3) * 3;// i know what it does.I don't know what are the principles behind it.
	//anyway it is still amazing!!
	for (int i = resizedrow; i < (resizedrow + 3); i++)
	{
		for (int j = resizedcolumn; j < (resizedcolumn + 3); j++)
		{
			sud.addition3x3box += sud.precious[i][j];
		}
	}
	sud.addition3x3box += sud.dummy[sud.row][sud.col];
	return 0;
}
int humaninput(int array[][length])
{
	while (true)
	{
		cout << "enter row, column and your number\n";
		cin >> sud.row >> sud.col >> sud.number;//if i put a letter all hell breaks loose

		if (sud.row < 0 || sud.row >= length)
		{
			cout << "out of bounds row\n";
		}
		else if (sud.col < 0 || sud.col >= length)
		{
			cout << "out of bounds column\n";
		}
		else if (sud.number < 0 || sud.number > length)
		{
			cout << "out of bounds number\n";
		}
		else if (sud.precious[sud.row][sud.col] != 0)
		{
			cout << "already assigned\n";
		}

		else break;
	}
	sud.dummy[sud.row][sud.col] = sud.number;

	return 0;// i was trying to return sud.dummy[][]. Wrongly thought it's out of scope and gone when i exit but struct are good
}
int machineinput(int array[][length])
{
	while (1)
	{
		sud.row = 0;
		sud.col = 0;
		sud.number = 0;
		sud.row = randomrow();
		sud.col = randomcol();
		sud.number = randomnumber();
		//just copy paste from human input
		if (sud.row < 0 || sud.row >= length)
		{
			cout << "out of bounds row\n";
		}
		else if (sud.col < 0 || sud.col >= length)
		{
			cout << "out of bounds column\n";
		}
		else if (sud.number < 0 || sud.number > length)
		{
			cout << "out of bounds number\n";
		}
		else if (sud.precious[sud.row][sud.col] != 0)
		{
			cout << "already assigned\n";
		}
		else break;
	}
		sud.dummy[sud.row][sud.col] = sud.number;

	return 0;
}
//it prints diagonal and in succession numbers. Dont understand it
int randomnumber()//the numbers follow a pattern unfortunately, i thought the delay will fix it
{
	unsigned int k = 0;
	this_thread::sleep_for(967ms);//maybe this number will help
	srand((unsigned)time(NULL));  //no idea how it works i will study it further
	k = (rand() % 9) + 1;
	cout << k<<"\n";
	return k;
}
int randomrow()
{
	unsigned int l = 0;
	this_thread::sleep_for(573ms);//dont really understand it. Stole it from a onelonecoder video
	srand((unsigned)time(NULL));
	l = rand() % 9;
	cout << l;
	return l;
}
int randomcol()
{
	unsigned int m = 0;
	this_thread::sleep_for(666ms);//curse you satan, not even you will help me
	srand((unsigned)time(NULL));
	m = rand() % 9 ;
	cout << m;
	return m;
}
void showthesudoku(int array[][length])
{
	for (int i = 0; i < length; i++)
	{
		for (int j = 0; j < length; j++)
		{
			cout <<  setw(3)<<array[i][j];
		}cout <<"\n";
	}
}
void thanksforplaying()
{
	cout << "thanks for playing";
}
Last edited on
A few things from a cursory glance,
(1) 'astruct' tells me no information about what that struct is for. You should give structs/classes a name related to what they actually contain or what they're supposed to do.
(2) You have 'sud' as a global variable. Prefer to have local variables that are passed to other functions.
(3) What is the purpose of the sleeping you have? Your sleep calls are suspiciously close to your srand calls... You don't need to call srand more than once. Just call it once, at the beginning of main before any main game loop. srand seeds the random number generator accessed through rand().
(4) You pass in a 'move' variable into your 'theinterface' function, but never use it. See, the use of global variables is already starting to create a mess, albeit a small one at this point.
(5) You appear to have a lot of functions that return things, but you don't use this return value anywhere. If you have a function that doesn't need to return a value, just make the return type void instead of int.
Last edited on
closed account (o35DwA7f)
The sleeping,i thought would break the pattern the random numbers have. I haven't really study rand, srand, and this thread. Yes i see the move variable argument had no meaning. I dont remember why i did it. I think i didn't really know how struct was working. But know i kind of understand it. The only reason i used struct was because i had problem passing values around different functions. I see what you say about void. The rest i can't understand at the moment. I will study. Thank you very much Ganado. I dont know why the code doesn't work on edit and run.I copy and paste the code above to visual studio and it works.
Last edited on
Topic archived. No new replies allowed.