HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Printing an N*N array in the rectangular/circular/spiral fashion

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thearraycircularprintingfashionrectangularspiral

Problem

The problem is to print an NN array and fill it with numbers(starting from 1 at (0,0)) in the rectangular/circular/spiral fashion, going inwards, ending at the NN value.

Also find and count the indices where the values are divisible by 11, in the circular fashion only.[(0,0) will always be an index]

What improvements can I make in the following code?

```
#include
#include

class arr{
int value;
bool property;
public:
arr(){
property=false;
}
void put_value(int a,bool b){
value = a;
property = b;
}
int get_value(){
return value;
}
bool get_property(){
return property;
}
};
int count = 1;
int counter(){
return count++;
}
int c=0;
int t(){
return c++;
}
class first{
int a,b;
bool flag;
public:
first(){flag=false;}
void put_value(int x, int y, bool m){
flag = true;
a = x ;
b = y;
}
int get_property(){return flag;}
int get_a(){return a;}
int get_b(){return b;}
};
int fill(int a, int b,int dimension ,arr** array,first cls[],int temporary){
int i=0,j=0,k=0,l=0;
int f=t();
int put;
for(i = b ; i =f; k-- ){
if(array[j][k].get_property()==false){
put = counter();
array[j][k].put_value(put,true);

if(!(put%11))
{ cls[temporary].put_value(j,k,true);
temporary++;
}
}
}
i=k+1;
for(l = j-1 ;l>=f ; l--){
if(array[l][i].get_property()==false){
put = counter();
array[l][i].put_value(put,true);

if(!(put%11))
{ cls[temporary].put_value(l,i,true);
temporary++;
}
}
}
return temporary;
};
int main(){
int dimension;
std::cin>>dimension;
arr** array = new arr*[dimension];
for(int i=0;i<dimension;i++)
array[i] = new arr[dimension];
int temp=dimension;
first kay = new first[dimensiondimension];
int temporary = 0;
for(int i=0;i<temp;i++){

Solution

Global Variables

The code contains the variables count and c declared as global variables. Global variables are generally frowned upon.

  1. As programs get more complex they are created in multiple files, global variables are very difficult to find when used in multiple files.



  1. Using global variables makes it harder to maintain the code.



  1. Using global variables makes it harder to debug any problems.



  1. Using global variables may prevent adding external libraries.



Variable Initialization in Constructors

In both of the classes the variables are initialized within the body of the constructor. The more accepted way to do this is this way:

first::first()
: flag{false}
{
}

arr::arr()
: property{false}
{
}


Use Descriptive Variable Names

@D.Jurcau point about variable and function names is correct.

The function fill() has a fairly meaningful name, it is filling a matrix. The function t() isn't clear at all.

The code should always be as self documenting as possible. This means that the variable and function names should be as descriptive as possible. If you or someone needs to come back and modify the code in 2 or 3 years the code needs to be instantly understandable.

Variable names like i, j, and k can be used in for loops, but it would make more sense to name them so that the algorithm is clear.

DRY Code

DRY stands for Don't Repeat Yourself. This is another item where D.Jurcau is correct. One of the ways to correct this is to put code into functions, another would be to use loops, which is already done.

Reducing repetition makes the code easier to write, debug, and maintain. Use the standard library and container classes whenever possible to reduce code.

Use the Single Responsibility Principal

When designing functions and objects (classes and structs) reduce the complexity of the functions by dividing large complex functions into smaller functions. This is called the Single Responsibility Principal.

Both the fill() function and main() are too complex.

Each of the loops in the fill() function should probably be separate functions. It is also unclear what each loop is doing in the fill() function. Having each loop be a function would better document what each loop is doing.

The purpose of the main() function is to set up the environment to run the program, execute the program and catch any exceptions, all other code such as input, output and actual execution should be in other functions. The actual logic for running the program should be in another function.

Additional Topics to Research

Knowing good programming principals is a good thing, additional things to research would be SOLID and KISS.

Code Snippets

first::first()
: flag{false}
{
}

arr::arr()
: property{false}
{
}

Context

StackExchange Code Review Q#136451, answer score: 4

Revisions (0)

No revisions yet.