School project, just wondering if there is anymore documentation that I woould need?

//********************************************************/
// Tim Petrich, T.Scot Alexander
// Program: Project 1 Mystery.cpp
// Date: 17 January 2007
// Purpose: Binary Convert
// Description: Converts text into binary using the division method
// through ASCII code.
// The program as compiled and executes using
// Microsoft Visual Studio C++. Net 2003 Version 7.1
#include "stdafx.h"
#include <iostream> // access input/output stream.
using namespace std;
#include <cstring> // to use strlen function
#include <cstdlib> // to use exit function
char *entry, letter, choice[2]
; //declare a char pointer, variable, and array
int ascii, len, binary[8], total;
void prog();
int main()
{
    prog();
    return 0;
}

void prog()
{
    entry = new char[501]; // allocating memory for entry
    /* entry should be dynamic, otherwise a new
    string entry of 501 chars would be created
    each time function is called!
    Talk about memory hog! */
    cout<<"Enter string to convert (up to 500 chars): "; // user prompt
    cin.getline(entry, 500); // gets upto 500 chars and stores in entry
    len = strlen(entry); /* get the number of characters in entry. */
    // this loop is executed for each letter in the string
    for(int i = 0; i<len; i++)
    {
        total = 0;
        letter = entry[i]; // store the first letter
        ascii = letter; // put that letter into an int, so we can
        // see its ASCII number */
        while(ascii>0) // This while loop converts the ASCII number into binary,
            // stores it backwards into the binary array
        {
            // To get the binary code one must take the decimal number in
            // question, take it and divide it by two repeatedly, save
            // the remainder (which will become the binary number), save
            // the whole number, divide by two, and repeat the whole
            // process until 0 is reached. This if-else statement serves
            // this functionality, by getting the remainder of the ASCII
            // code, storing it in the array and then dividing the int
            // ascii by two
            if((ascii%2)==0)
            {
                binary[total] = 0;
                ascii = ascii/2;
                total++; // increasing by one each time will yield the
                // number of numbers in the array.
            }
            else
            {
                binary[total] = 1;
                ascii = ascii/2;
                total++;
            }
        }
        total--; // due to data type factors, the program will actually
        // add a 0 at the end of the array that is not supposed
        // to be there, decrementing total will solve this
        // problem, as that 0 will not be displayed.
        // this while loop displays the binary code for each letter.
        while(total>=0)
        {
            cout<<binary[total];
            total--;
        }
    }
    delete[] entry; // free up the memory used by entry
    cout<<endl<<"Do again(1 = yes, 2= no)?: "; // user prompt
    cin.getline(choice,3);
    if(choice[0] == '1')
        prog(); // a recursive call to the function. a function loop.
    else
        exit(0);// terminates program, properly closes all files
}

It looks good to me - the comment blocks explain everything pretty well.

(Especially compared to my projects; where commenting is quite infrequent.)

Actually too much documentation, I would say that a complete overkill. You just need to provide a short abstract on what your algorithm does rather than clubbing the program to death by explaining each and every statement.

I don't know if you have reflected on this but almost 50% of your code is comments which is actually a bad thing...

I have a boss once that required comments on each and every line of code. He was not proficient in computer languages but wanted to read and understand their logic. Now that is overkill.

School project, just wondering if there is anymore documentation that I woould need?

You have too much. I'll go through each of your comments and describe why. :) The neat thing is that weak comments can be spotted even without looking at the code that's being commented. ;)

//********************************************************/
// Tim Petrich, T.Scot Alexander
// Program: Project 1 Mystery.cpp
// Date: 17 January 2007
// Purpose: Binary Convert
// Description: Converts text into binary using the division method
// through ASCII code.
// The program as compiled and executes using
// Microsoft Visual Studio C++. Net 2003 Version 7.1

The source header shouldn't be too verbose. Only the essentials is the order of the day. You also need to consider the future as well. If the comment doesn't scale well for maintenance, it's a weak comment.

// access input/output stream.

Looking at the code, this comment is inaccurate. You're not accessing the input/output stream, whatever that it. You're including a header to make standard stream names visible. Everyone who needs to read your code knows what iostream is for, you don't need to comment a header unless you use it for a very specific and possibly obscure reason.

// to use strlen function

The first question I would ask is "do you use anything other than strlen? how likely is it that you will?" This is a scalability thing, and if you're using C strings, you'll probably end up using more than strlen and your comment suddenly becomes misleading and inaccurate.

// to use exit function

Ditto. :)

//declare a char pointer, variable, and array

I know right away that this is a weak comment because it comments the mechanics and not the intention. The mechanics are obvious just by reading the code itself, but I might not know why you chose the mechanics, and that's what matters.

// allocating memory for entry

The pointer is called entry, and any reader will know that you're allocating memory. So this comment just repeats the code and now you have a maintenance nightmare because you need to keep the code and comment in sync.

/* entry should be dynamic, otherwise a new
string entry of 501 chars would be created
each time function is called!
Talk about memory hog! */

This is a pretty good comment. It tells me why you did what you did. It's prompted by a flawed solution, so you can get rid of it by improving the code, but the comment itself works okay.

// user prompt

Obviously. :) There's no need for a comment.

// gets upto 500 chars and stores in entry

There's no intention here, only mechanics.

/* get the number of characters in entry. */

This isn't quite accurate because it gets the length of the string rather than the number of characters. I would much rather know why you need the length than a rehash of the code into human speak. :)

// this loop is executed for each letter in the string

Once again, this is obvious. It's even more obvious because the loop condition uses the length that you just retrieved. If those two parts were far apart, a comment on what the loop does is warranted, but the comment should be more abstract.

// store the first letter

The first letter of what?

// put that letter into an int, so we can
// see its ASCII number */

This tells me why, but you really don't need to do that. A char is just a small int already. ;)

// This while loop converts the ASCII number into binary,
// stores it backwards into the binary array

This is a decent comment, but it could be more abstract.

// To get the binary code one must take the decimal number in
// question, take it and divide it by two repeatedly, save
// the remainder (which will become the binary number), save
// the whole number, divide by two, and repeat the whole
// process until 0 is reached. This if-else statement serves
// this functionality, by getting the remainder of the ASCII
// code, storing it in the array and then dividing the int
// ascii by two

You just told me the same thing that the code does, just with more words. ;)

// increasing by one each time will yield the
// number of numbers in the array.

The number of numbers? That's not very informative.

// due to data type factors, the program will actually
// add a 0 at the end of the array that is not supposed
// to be there, decrementing total will solve this
// problem, as that 0 will not be displayed.

The comment is good, but you're yapping too much. Keep it short and sweet unless you have no other choice.

// this while loop displays the binary code for each letter.

A comment is okay, but it should be more abstract.

// free up the memory used by entry

That's obvious from the code. You don't need to repeat it.

// user prompt

Why yes, it is. ;)

// a recursive call to the function. a function loop.

This is a great place to explain in detail why you picked recursion instead of a loop.

// terminates program, properly closes all files

What files? Why do I need to know what exit() does?


I restrained myself and only changed the comments in your code. Here's the result.

/*
  Binary Convert (Project 1, Mystery.cpp)

  17/1/2007 - Tim Petrich, T. Scot Alexander (MSVS .NET 7.1)
*/
#include "stdafx.h"
#include <iostream>
using namespace std;
#include <cstring>
#include <cstdlib>
char *entry, letter, choice[2];
int ascii, len, binary[8], total;
void prog();
int main()
{
  prog();
  return 0;
}

void prog()
{
  // entry is dynamic to save memory for recursive calls
  entry = new char[501];

  cout<<"Enter string to convert (up to 500 chars): ";
  cin.getline(entry, 500);
  len = strlen(entry);

  // Translate the string to binary
  for(int i = 0; i<len; i++)
  {
    total = 0;
    letter = entry[i];
    ascii = letter;
    
    // Translate one character
    while(ascii>0)
    {
      if((ascii%2)==0)
      {
        binary[total] = 0;
        ascii = ascii/2;
        total++;
      }
      else
      {
        binary[total] = 1;
        ascii = ascii/2;
        total++;
      }
    }

    // Translation leaves an extra 0. Ignore it.
    total--;

    // Display the translated character
    while(total>=0)
    {
      cout<<binary[total];
      total--;
    }
  }

  // Free memory before the recursive call
  delete[] entry;

  cout<<endl<<"Do again(1 = yes, 2= no)?: ";
  cin.getline(choice,3);
  if(choice[0] == '1')
    prog();
  else
    exit(0);
}

There are a good 70% fewer comments, but none of the most valuable information is lost. If I were more intrusive, I would do something like this. ;)

/*
  Binary Convert (Project 1, Mystery.cpp)

  17/1/2007 - Tim Petrich, T. Scot Alexander (MSVS .NET 7.1)
  18/1/2007 - Raye Cutsin (MSVS 2005 8.0)
*/
#include <iostream>
#include <cstring>
#include <cstdlib>


namespace {
  // Size assumptions
  const int BYTE_SIZE = 8;
  const int BUFFER_SIZE = 501;
}


void binary_convert( char buffer[], size_t size );


int main()
{
  using namespace std;

  char buffer[BUFFER_SIZE];

  while ( true ) {
    binary_convert( buffer, sizeof buffer );

    cout << "\nDo again(1 = yes, 2= no)?: ";
    cin.getline( buffer, sizeof buffer );

    if ( buffer[0] != '1' )
      break;
  }

  return 0;
}


void binary_convert( char buffer[], size_t size )
{
  using namespace std;

  cout << "Enter string to convert (up to 500 chars): ";
  cin.getline( buffer, size );
  
  // Translate the string to binary
  for ( size_t i = 0; buffer[i] != '\0'; i++ ) {
    size_t total = 0;
    int binary[BYTE_SIZE];

    // Translate and display one character
    for ( char letter = buffer[i]; letter > 0; letter /= 2 )
      binary[total++] = ( letter % 2 ) != 0;

    while ( --total > 0 )
      cout << binary[total - 1];
    cout << ' ';
  }
}

Commenting is as much of an art form as programming. Just keep on practicing and you'll get a good feel for how you best comment. :)

Update: Btw what does the program exactly do ? Convert a number to binary is it ?

I am unsure because for the input 1 the output given is 110001 which is rather err..incorrect for a binary representation of 1 . Is this what you intended wheelz ? Ditto with Raye's code ? Am I missing something here...?

Update: Btw what does the program exactly do ? Convert a number to binary is it ?

I am unsure because for the input 1 the output given is 110001 which is rather err..incorrect for a binary representation of 1 . Is this what you intended wheelz ? Ditto with Raye's code ? Am I missing something here...?

I assumed that it was a translation of the ascii value to binary. '1' in ascii is 31H and the binary translation is 00110001. Leading zeros are snipped with this translation method, so you would get 110001. That's just a guess though. :)

Comments
always great comments and help
This question has already been answered. Start a new discussion instead.