C++

Forum

 
Lo Shu Magic Square with Three Functions

Angrboda (7)
Hello everyone!

The specifications of my current CS project (Intro to Programming using C++)
have left me a bit stumped I am afraid. It is a Lo Shu magic square program. The program is supposed to have at a minimum three functions, one to "open the file, one to evaluate if the square is a magic square, and a function to output the results."

Where I'm lost is how exactly can I display the correct output without embedding the function call for output() at the end of the signature of magicSquare() ? (of course see code below).

Below is my first submission and the professor's notes follow:

------------------------------------------------------------------------------
Don't call output multiple times - too inefficient.

Program crashes. Did you get any warning messages? You should fix the problems in those warnings. Here is what I got:

c:\topicf.cpp(31): warning C4700: uninitialized local variable 'flag' used


topicf.cpp(60): warning C4715: 'openFile': not all control paths return a value
------------------------------------------------------------------------------

I'm not sure how to properly associate the correct iteration of the loop in magicSquare() with the result of the control structure that stores true or false in the boolean variable flag with the output() function so that output() displays the output correctly without embedding the function call at the end of the for loop in magicSquare(). I'm not worried about the compiler warnings I've solved those problems already.

The file is a 7 x 9 matrix of ints, each row containing 1 - 9 in seemingly random order, which most of you could probably tell anyway after examining the source code.

Please don't just give me the answer; I would truly prefer an explanation that will lead me to the right solution.

Thanks a lot, you guys. 



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
 

#include <iostream>
#include <fstream>

using namespace std;

const int ROW = 7;                                  // Global constants for array dimensions.
const int COL = 9;

bool openFile( int[][COL] );                        // Opens file and reads data into 2D array. Bool return type for error checking.
void magicSquare( int[][COL], bool, int );          // Performs arithmetic to check if the necessary elements add up to 15.
void output( int[][COL], bool, int );               // Outputs the matrices and corresponding statements (magic square or not magic square).
	                                            // Note: the call to output() is embedded in the signature of the  magicSquare() function. 
int main()
{   
	int row = 0;
	int nums[ROW][COL]; 
	                                            // 2D array is declared and properly sized for the Lo Shu magic square exercise. 
	bool flag = true;                           // two booleans, one for output and one for error checking. 
	bool flag2 = true;

	flag2 = openFile( nums );		    // flag2 stores return value from openFile for error checking (see lines 44 - 58).

	if ( flag2 )				    // if flag2 is true (if there are no problems opening the file), call magicSquare(). 
	magicSquare( nums, flag, row );

	

	cout << "Press enter to end -->  ";
	cin.get();

	return 0;
}

bool openFile( int nums[][COL] )				// Function to open "TopicFSq.txt" and read data to array.
{
	ifstream inputFile;
	inputFile.open( "TopicFSq.txt" );

	if ( inputFile )					// If the file is successfully opened, store the data in an array.
	{
		for ( int row = 0; row < ROW; row++ )
		{
			for ( int col = 0; col < COL; col++ )
			{
				inputFile >> nums[row][col];
			}
		}
		inputFile.close();
	}
	else			                             // If the file is not opened successfully output error message and return false to flag2 in main
	{				                     // in order to skip magicSquare() and end the program.
		cout << "Error opening file.\n" << endl;
		return false;
	}	
}

void magicSquare( int nums[][COL], bool flag, int row )				   // Function to test if the square satisfies the necessary conditions 
{								                   // to be a Lo Shu magic square.
	for ( int row = 0; row < ROW; row++ )
	{
	   if ( nums[row][0] + nums[row][1] + nums[row][2] == 15 &&
		nums[row][3] + nums[row][4] + nums[row][5] == 15 &&
		nums[row][6] + nums[row][7] + nums[row][8] == 15 &&
		nums[row][0] + nums[row][3] + nums[row][6] == 15 &&		   // For each row in the array made into a 3 x 3 square, the elements must 
		nums[row][1] + nums[row][4] + nums[row][7] == 15 &&		   // add up to 15 vertically, horizontally, and diagonally. 
		nums[row][2] + nums[row][5] + nums[row][8] == 15 &&
		nums[row][0] + nums[row][4] + nums[row][8] == 15 &&
		nums[row][2] + nums[row][4] + nums[row][6] == 15 )
		flag = true;						           // If the above comment's predicate is satisfied, store true in flag.
	    else							           // If the predicate is not satisified, store false in flag.
		flag = false;	

	   output(nums, flag, row);
	}															       
}																	   

void output( int nums[][COL], bool flag, int row )							// Output function
{
	
	cout << nums[row][0] << "  " << nums[row][1] << "  " << nums[row][2] << endl;			// Output is formatted to 3 x 3 sqaures 
	cout << nums[row][3] << "  " << nums[row][4] << "  " << nums[row][5] << endl;
	cout << nums[row][6] << "  " << nums[row][7] << "  " << nums[row][8];

	if ( flag )
		cout << "  The square IS a Lo Shu magic square.\n" << endl;	                 // If boolean identifier flag stores true from 
	else                                                                                     // control structure in magicSquare()
		cout << "  The square is NOT a Lo Shu magic square.\n" << endl;		         // If flag stores false
	                                                                                          
}
Last edited on
shadder (813)
Hi,

1
2
3
4
5
6
7
8
 
   if ( nums[row][0] + nums[row][1] + nums[row][2] == 15 &&
		nums[row][3] + nums[row][4] + nums[row][5] == 15 &&
		nums[row][6] + nums[row][7] + nums[row][8] == 15 &&
		nums[row][0] + nums[row][3] + nums[row][6] == 15 &&		   // For each row in the array made into a 3 x 3 square, the elements must 
		nums[row][1] + nums[row][4] + nums[row][7] == 15 &&		   // add up to 15 vertically, horizontally, and diagonally. 
		nums[row][2] + nums[row][5] + nums[row][8] == 15 &&
		nums[row][0] + nums[row][4] + nums[row][8] == 15 &&
		nums[row][2] + nums[row][4] + nums[row][6] == 15 )


this is a very unhealty way to deal with lo shu (aka magic square)

you have to use some nested for loops for the algorithm

I am pretty sure you have done some matrix exercises, you can implement that

---------------------------------------------

there are many other way to implement it, a more efficient one is

https://en.wikipedia.org/wiki/Magic_square#Types_of_construction
Angrboda (7)
I sorted the issue out. The trick is to declare a boolean array, pass it to both magicSquare and output functions, and store true or false in each element of the array using an if / else construct right after that horrifying monstrosity of a test in the magicSqaure function, while also using an if / else construct similarly for the output function to ensure that what is displayed is correct.

I would love a bit more of a push in the right direction as far as sorting out that oafish dilapidated rookie novice tard of a test in the magicSquare function.

I haven't really refined my mathematical reasoning abilities quite up to that point yet.

See the updated code below

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
 
#include <iostream>
#include <fstream>

using namespace std;

const int ROW = 7;                                         
const int COL = 9;

bool openFile( int[][COL] );                            
void magicSquare( int[][COL], bool[] );           
void output( int[][COL], bool[] );                     
												  
int main()
{
	int nums[ROW][COL];					        
	bool check[ROW];							   
	                                               
	bool flag = true;                                                        
	

	flag = openFile( nums );	                                          

	if ( flag )									   
	{
		magicSquare(nums, check);				  

		output(nums, check);					  
	}
	else
		cout << "Error opening file." << endl;	           

	cout << "Press enter to end -->  ";
	cin.get();

	return 0;
}

bool openFile( int nums[][COL] )				                      
{
	ifstream inputFile;
	inputFile.open( "TopicFSq.txt" );

	if ( inputFile )					                                     
	{
		for ( int row = 0; row < ROW; row++ )
		{
			for ( int col = 0; col < COL; col++ )
			{
				inputFile >> nums[row][col];
			}
		}
		inputFile.close();
		return true;
	}
         else      
                return false;
}

void magicSquare( int nums[][COL], bool check[] )				       
{								                                               
	for ( int row = 0; row < ROW; row++ )
	{
		
		if ( nums[row][0] + nums[row][1] + nums[row][2] == 15 &&
		     nums[row][3] + nums[row][4] + nums[row][5] == 15 &&
		     nums[row][6] + nums[row][7] + nums[row][8] == 15 &&
		     nums[row][0] + nums[row][3] + nums[row][6] == 15 &&		   
		     nums[row][1] + nums[row][4] + nums[row][7] == 15 &&		
		     nums[row][2] + nums[row][5] + nums[row][8] == 15 &&
		     nums[row][0] + nums[row][4] + nums[row][8] == 15 &&
		     nums[row][2] + nums[row][4] + nums[row][6] == 15 )

		     check[row] = true;						                                
	       else							                                 
		     check[row] = false;										 
	}
}

void output( int nums[][COL], bool check[] )						             
{
	for ( int row = 0; row < ROW; row++ )
	{
		for ( int col = 0; col < COL; col++ )
		{
			cout << nums[row][col] << "  ";

			if (col == 2)
				cout << endl;					
			if (col == 5)
				cout << endl;
			if (col == 8)
			{
				if ( check[row] )												
					cout << " The square IS a Lo Shu magic square.\n" << endl;
				else
					cout << " The square is NOT a Lo Shu magic square.\n" << endl;		 
			}
		}
	}
}
Last edited on
bird1234 (126)
Program is working fine no error update your compiler
Angrboda (7)
I am aware that it works. It's just that I am looking for a more elegant solution to that conditional in the magicSquare function.
jdm1891 (44)
A few pointers:

*You are using way too many magic numbers in your code.

*Comment your code! This combined with the reason above makes it very hard to read and understand

*I would recommend, if you are using C++11, to use an std::array over a normal array, or even an std::vector

*Your use of the check array makes no sense to me: why would you need to know if each separate row adds to 15, if you only ever check the eighth variable?

*I would recommend you to try to hard code as little as possible and generalise the problem, by this I mean, to be able to change as little as possible in your code if your professor suddenly ask for the grid to be a different size or for the numbers to add up to a different value

*In my opinion, you should make your magicSquare return the variable instead of assigning it in the function

This is an example program that follows the above guidelines that I made, note that I made this quickly and is NOT the program you were asked to create, so copying it would be useless. However, if you look at some of the things I did you could adapt it and integrate the ideas behind it into your own code - which is a very important skill.

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
 
//for std::cout and std::endl
#include <iostream>
//For std::vector
#include <vector>
//For assert()
#include <cassert>

#define SUM 15

bool magicSquare(const std::vector<std::vector<int> >& nums);

int main() 
{
	//Here as you can see I am using vectors, this is simply because vectors do not need to be hard coded with a size
	std::vector<std::vector<int> > nums = 
	{
		{8, 1, 6},
		{3, 5, 7},
		{4, 9, 2}
	};

	//Of course this means that we need to make sure the vectors are valid size, this simply throws an error if the input is not square
	for (const auto& inner : nums)
	{
		assert(inner.size() == nums.size());
	}

	//This is our output
	std::cout << "The square: " << std::endl;

	//This is a C++11 feature(I think), it is just a for each loop
	//It prints the magic square that was inputted
	for (const auto& row : nums)
	{
		for (const auto& number : row)
		{
			std::cout << number << " ";
		}
		std::cout << std::endl;
	}

	//Note I intergreate my function into the if statement here which, in my opinion, makes it look nicer
	if (magicSquare(nums))
	{
		std::cout << "is magic!" << std::endl;
	}
	else
	{
		std::cout << "isn't magic!" << std::endl;
	}

	//never use system() in your code, I just did this for quickness
	system("pause");
	return 0;
}

bool magicSquare(const std::vector<std::vector<int> >& nums)
{
	//these are our variables which need to be declared before use
	int sum_column = 0;
	int sum_row = 0;
	int sum_diagonal_left = 0;
	int sum_diagonal_right = 0;
	

	//here I go through each column
	for (std::size_t current_column = 0; current_column < nums.size(); ++current_column)
	{
		//here I go through each loop
		for (std::size_t current_row = 0; current_row < nums.size(); ++current_row)
		{
			//here I take my sums
			//Note that here I have sacrificed efficiency for more 'elegant' and 'pretty' code. A more efficient way of doing it would to have separate for loops for every sum, and have some for loops increase by different amounts, and only do each once.

			//This sums every number in the row
			sum_row += nums[current_column][current_row];
			
			//this sums every number in the column, by swapping the current_number and current_row variables
			// so instead of going
			// 1,1
			// 1,2
			// 1,3
			// it instead goes
			// 1,1
			// 2,1
			// 3,1
			sum_column += nums[current_row][current_column];

			//this sums the diagonal by just taking the current row twice
			//so like this:
			// 1,1
			// 2,2
			// 3,3
			sum_diagonal_left += nums[current_row][current_row];

			//These diagonal sums are the reason this is inefficient, they only need to be calculated once, yet they are calculated every loop. this could easily be fixed by putting an if(current_column==0) at the beginning of these lines though
			//this takes the diagonal the other way. nums.size()-current_row(so it goes down instead of up
			//the one is because the size() function starts at one but the indexing starts at 0
			//not is is NOT like this:
			// 3,3
			// 2,2
			// 1,1
			// Only the first digit must be inverted because as you may have noticed this is just the last one backwars
			//which would give the same result
			//it should be like this:
			// 3,1
			// 2,2
			// 1,3
			sum_diagonal_right += nums[(nums.size() - current_row - 1)][current_row];
		}
		//note I have not used a 'magic number' here, like you did
		if (sum_row != SUM || sum_column != SUM || sum_diagonal_left != SUM || sum_diagonal_right != SUM)
		{
			//return false, one of the values did not add up
			return false;
		}

		//here I am just resetting the sums
		sum_row = 0;
		sum_column = 0;
		sum_diagonal_left = 0;
		sum_diagonal_right = 0;
	}

	//if everything added to 15, return true
	return true;
}


Last edited on
Angrboda (7)
I couldn't seem to get the comments in there without it looking like a complete mess in the second version and not having to rewrite them all out differently which would have just been too time consuming at that juncture.

Wow. Thank you for putting the effort that you have into your response.

Yes, the best solution would be one that works in general. I most certainly agree. The exercise was taken out of our text and modified by the professor to require the use of three separate functions, as I had noted in the first post.

He also provided us a .txt file that is a 7 x 9 array; a file with 7 rows of 9 columns of numbers
( 1 - 9 randomized ), and provided us with an .exe so we could see some output and asked us to match his output.

From looking at his output i deduced that his code checked each row of 9 numbers formatted into a 3 x 3 magic square, which I just wouldn't know how to generalize a solution like that seeing as though i wrote my first very simple program three months ago when i started this journey.

I really see a lot of helpful concepts in your example that you have illustrated. Thanks a lot. I can take a fair amount of knowledge away from your help. I was really looking for a general solution to this problem because of the utility inherent in that kind of programming. That is what I want to learn for certain.
Topic archived. No new replies allowed.