Code Critique Please

[Rv5]

I have an assignment due mid next week that I have completed. I was hoping
someone could take a look at the code and tell me what they think of the
style. Id like to know if this is good code that I can be proud of or if my
technique still needs some work. What can be improved? I created an htm
page that first lists the assignment, and under that is my code. I think
the code is good, but Ive thought that before...

http://www.69chargerrt.com/comp322.htm

Thanks
Rv5

[Rv5]

One thing I forgot to mention is that I need not be concerned with input
errors. The teacher said not to bother programming any catches and such to
trap input errors, so please keep that in mind when viewing.

Thanks
Rv5

[Phlip]

Rv5 wrote:

I have an assignment due mid next week that I have completed. I was hoping someone could take a look at the code and tell me what they think of the
style. Id like to know if this is good code that I can be proud of or if my technique still needs some work. What can be improved? I created an htm
page that first lists the assignment, and under that is my code. I think
the code is good, but Ive thought that before...

http://www.69chargerrt.com/comp322.htm
In future, please post your source here, and skip the assignment question.
Either report a bug, syntax error, a design you can't achieve, or a request
about style.

Warning: So many kids ask this newsgroup "please do my homework for me" that
questions about homework should be phrased carefully.
#include <iostream>
#include <cstdlib>
#include <cstdio>
#include <string>
using namespace std;
That line defeats the purpose of namespaces. Write

using std::string;

etc. for each individual identifier you want to import. You'l find
surprisingly few of them.
int f; // max number of frames
That kind of comment is a "code smell" that the indentifier it comments
could have a better name. Try:

int max_number_of_frames;
int l; // length of reference string
string r; // reference string
Give variables the narrowest scope possible. These shouldn't be outside the
functions that use them.
int numfaults = 0; // number of page faults
char *frame; // frames that will contain the letters
int *counter; // counts letter time in the frame since last referenced
Students should start with STL and container classes like std::vector. They
should not need pointers for the first several assignments. Read
/Accelerated C++/ to learn these features in the right order.
bool analyzeFrames(int i)
{
for (int k = 0; k < f; k++)
{
// first trys to find the page in the frame
if (frame[k] == r[i])
{
This function is too long. A good place to break it is one of the inner
controlled block. Everything this 'if' controls could be inside a subsidiary
function.
counter[k] = 1; // finds the letter, resets its counter
for (int l = k+1; l < f; l++) // loaded pages time in frame continues counter[l]++; // increment other page counters
return true;
}
// if the page is not loaded, see if there is a blank frame to load it in else if (frame[k] == '-')
{
frame[k] = r[i]; // fill the empty cell with next page
counter[k] = 1; // set that page counter to 1
numfaults++; // page was not in the frame, so a page fault occurs
return true;
}
else
counter[k]++; // increments counter, page not used this time
}
numfaults++; // page not found and could not be loaded, page fault occurs return false;
}

void replace(int i)
{
// largest counter index is the frame that gets replaced
int largest = counter[0];
int index = 0;

// loop through counter array
for (int k = 1; k < f; k++)
if (counter[k] > largest) //finds the largest counter
{
largest = counter[k];
index = k;
}

frame[index] = r[i]; // replaces least recent page with new one
counter[index] = 1; // sets new page counter to 1
}

int main()
{
// user enters values of variables
This function is also way too long. The first block of it, the input stuff,
could be inside a function called user_enters_values_of_variables().
cout << "Input maximum number of frames (f): ";
cin >> f;

cout << "Input length of reference string (0 < l < 101): ";
cin >> l;

cout << "Input reference string (r): ";
cin >> r;
Duplicated code (like this cout-cin sequence) is a sign there could be a
function. In this case, the function could call like this:

f = getValue("Input maximum number of frames (f): ");
// displays the values the user entered
cout << "f = " << f << endl;
cout << "l = " << l << endl;
cout << "r = " << r << endl;

// creates the proper number of cells in the frame and counter arrays
frame = new char[f];
counter = new int[f];
Nobody deletes these. Use std::vector (even if your professor did not cover
them yet). If in the rare chance you actually need an array, use 'delete[]
frame' to release the storage when you are done with it.
// fills the frame and counter arrays with '-' and 0 respectively
for (int i = 0; i < f; i++)
{
frame[i] = '-';
counter[i] = 0;
cout << frame[i]; // verifies that the frame is empty
}
cout << endl;

// main loop that is going through the reference string
for (int i = 0; i < l; i++)
{
if(!analyzeFrames(i)) // attempts to find page or blank cell to load it to {
replace(i); // not found, no empty cells, replace least recent page
}
// displays contents of frame as the program runs
for (int l = 0; l < f; l++)
cout << frame[l];
cout << endl;
}

cout << "Total page faults = " << numfaults << endl;

return 0;
}

Like I used to say, as a lab aide, when finishing attending to a student:
"Good luck!"

--
Phlip

[Benny Hill]

On Sat, 15 Nov 2003 16:20:33 -0800, Rv5 wrote:

http://www.69chargerrt.com/comp322.htm

Thanks
Rv5

This URL is not accessible. I get the error message:

"Access to the port number given has been disabled for security reasons"
--
Benny
Remove your rose colored glasses before e-mailing me