Wanna help me revise my code?

[carrja99]

/**************************************************
* Description: A program that reads in graph edges from
* a designated input file. The input file should contain
* two adjacent alpha characters for each edge that may or
* may not be seperated. Using these edges the program will
* then build an adjacency matrix, and then prompt the user
* for an output file to write the adjacency matrix to.
************************************************** ***/
#include <iostream>
#include <fstream>
using namespace std;

void sort_array(int a[], int size)
{
int temp = 0, j = 0;
for (int i = 0; i < size -1; i++)
{
j = i;
while (a[j] > a[j+1])
{
temp = a[j];
a[j] = a[j+1];
a[j+1] = temp;
if(j != 0)j--;
}
}
}
void read_from_file(ifstream &read, char a[], int &size)
{
char c;
while(read >> c)
{
if (isalpha(c))
{
a[size] = toupper(c);
size++;
}
}
}




int main(void)
{
int size = 0, matrix[26][26];
ifstream read; ofstream write;
char c, filename[15], ofilename[15], a[100];
bool exists = true;

cout << "Enter an input file to read from: ";
cin >> filename;

read.open(filename);

if(read.fail())
{
cout << "File opening failed!" <<endl;
exit(1);
}

read_from_file(read, a, size);

/*************************************************/
for(int i = 0; i < 26; i++)
{
exists = false; // specifies if a vertice exists
int k = 0, b[26]; // b will hold what vertices the one pointed to is connected to
// e.g. AB, AC, AD would yield b[] = {0,1,2,3}
c = i + (int)('A'); //sets the char to search for

for (int j = 0; j < size; j++) // find connections
{
if (a[j] == c) //evaluates to true if the char is a vertice
{
exists = true;
if (k == 0)
{
b[k] = (int)a[j] - (int)('A'); //sets the 1st value as it's own value
k++;
}
if(j%2 == 0) // if it is the 1st vertice in the pair, the 2nd vertice
// is put in the array. (e.g. AB would put 1 in the array)
{
b[k] = (int)a[j+1] - (int)('A');
k++;
}
else // if it is the 2nd vertice, add the 1st vertice to the
// array. (e.g. in FA, 5 would go in the array)
{
b[k] = (int)a[j-1] - (int)('A');
k++;
}
}

} // end nested for-loop
sort_array(b, k); // sort the array for simplicity

int p = 0; // a temporary array pointer
for(int j = 0; j < 26; j++)
{
// if it exists, if j is a connection, and the end of the array hasnt been read
if ((exists == true)&&(j == b[p])&&(p < k)) // only prints 1s to matrix if it exists
{ // avoids non-existant lines having the
matrix[i][j] = 1; // same data as the last existant line
p++;
}
else
matrix[i][j] = 0;
}
}
/*************************************************/
cout << "Enter an Output file to write to: ";
cin >> ofilename;
write.open(ofilename);

/**** write the chars at the top of the file ***/
write << " ";
for (int i = 0; i < 26; i++)
{
write << (char)((int)('A') + i) << " " ;
}
write << endl;

/**** Write the matrix to the file ****/
for (int i = 0; i < 26; i++)
{
write << (char)(i + (int)('A')) << " ";
for (int j = 0; j < 26; j++)
{
write << matrix[i][j] << " ";
}
write << endl;
}
cout << "Output file " << ofilename << " has been writen." <<endl;
write.close();
read.close();

return 0;
}

[Sepero]

Perhaps this should have been posted in the programming/scripting section?

[cjanscen]

Change the increments and decrements to prefix, so:

i++; -> ++i;

Saves 4 bytes of memory every instance, unless the compiler doesn't already do that. The post-increment makes a temporary copy of the original value, then increments the variable and returns the temporary copy...like:

i=10;
j=0;

j =i++;

now j=10, and i=11 ... thats all fine but when you say:

for (i=0; i <100; i++) {

// code
};

there were 100 temporary copies of i created that nothing was done with, they were a waste.

..and thats my pointless C/C++ tip of the day!

[cjanscen]

You might want to change this:

for (int i = 0; i < 26; i++)
{
write << (char)(i + (int)('A')) << " ";
for (int j = 0; j < 26; j++)
{
write << matrix[i][j] << " ";
}
write << endl;
}

to something like this:

for (int i =0, int j =0; i <26; ++i, ++j) {

write << (char)(i + (int)('A')) << " ";
write << matrix[i][j] << " ";
write << endl;
}

This will save 26 comparisons .....other cleanups might be possible to, but besides that it looks good.

[banzaikai]

Hey!

You call this programming?

I couldn't find a single GOTO or GOSUB in the whole thing :D

cjanscen must've cut his/her teeth on a VIC-20 - going for the optimal "crunch" factor.

Humor aside, I like the code's cleanup/confirmation at the end - most programmers forget that part (along with the section that re-organizes for simplicity - humans gotta read this stuff, ya know).

The only error I can find is that "seperate" should be spelled "separate".


banzai "One of these days, I'm gonna learn C..." kai
"What language do you program in?"
"English."