tags:

views:

185

answers:

5

can anyone help me make this more generalised and more pro?

#include <fstream>
#include <iostream>
#include <string>
#include <vector>

using namespace std;


int main()
{
 // open text file for input: 

 string file_name;

 cout << "please enter file name: ";
 cin  >> file_name;

 // associate the input file stream with a text file

 ifstream infile(file_name.c_str());

 // error checking for a valid filename

 if ( !infile )
    {
  cerr << "Unable to open file "
    << file_name << " -- quitting!\n";
  return( -1 );
    }
  else cout << "\n";

 // some data structures to perform the function

 vector<string> lines_of_text;
 string textline;

 // read in text file, line by 

 while (getline( infile, textline, '\n' ))
 {
  // add the new element to the vector

  lines_of_text.push_back( textline );

  // print the 'back' vector element - see the STL documentation

  cout << lines_of_text.back() << "\n";
 }

 cout<<"OUTPUT BEGINS HERE:  "<<endl<<endl;
cout<<"the total capacity of vector: lines_of_text is: "<<lines_of_text.capacity()<<endl;

int PLOC = (lines_of_text.size()+1);
int numbComments =0;
int numbClasses =0;

cout<<"\nThe total number of physical lines of code is: "<<PLOC<<endl;


for (int i=0; i<(PLOC-1); i++)

//reads through each part of the vector string line-by-line and triggers if the 

//it registers the "//" which will output a number lower than 100 (since no line is 100 char long and if the function does not 
//register that character within the string, it outputs a public status constant that is found in the class string and has a huge value
//alot more than 100.


{
string temp(lines_of_text [i]);
if (temp.find("//")<100)
numbComments +=1;
}
cout<<"The total number of comment lines is: "<<numbComments<<endl;

for (int j=0; j<(PLOC-1); j++)
{
string temp(lines_of_text [j]);
if (temp.find("};")<100)
numbClasses +=1;
}
cout<<"The total number of classes is: "<<numbClasses<<endl;
A: 

Don't import the whole std namespace, only things you need from it:

using std::string;

Use a consistent naming convention: decide whether you prefer name_for_a_variable or nameforavariable or nameForAVariable. And use meaningful names: numbComments makes me associate to very different things than would numberOfComments, numComments or commentCount.

If your original code looks like this, I strongly recommend to select a single consistent indentation style: either

if ( ... )
    {
    ...
    }

or

if ( ... )
{
    ...
}

bot not both in the same source file.

Also remove the useless comments like

// add the new element to the vector

This is "only" about the readability of your code, not even touching its functionality... (which, as others have already pointed out, is incorrect). Note that any piece of code is likely to be read many more times than edited. I am fairly sure that you will have trouble reading (and understanding) your own code in this shape, if you need to read it even a couple of months after.

Péter Török
cool, I'll do that
ace
A: 

"More professional" would be not doing it at all. Use an existing SLOC counter, so you don't reinvent the wheel.

This discussion lists a few: http://discuss.techinterview.org/default.asp?joel.3.207012.14

Another tip: Don't use "temp.find("};}) < 100)", use "temp.find("};") != temp.npos;"

Edit: s/end()/npos. Ugh.

Stephen
You mean `temp.npos`. An annoying inconsistency in the STL.
Thomas
yeah, unless i have to for a project...:)
ace
Indeed, annoyingly npos. Thanks, fixed.
Stephen
+3  A: 

I'm not really sure what you're asking, but I can point out some things that can be improved in this code. I'll focus on the actual statements and leave stylistic comments to others.

  • cin >> file_name;
    To handle file names with spaces, better write
    getline(cin, file_name);

  • int PLOC = (lines_of_text.size()+1);
    Why do you claim that there's one more line than there actually is?

  • if (temp.find("//")<100)
    with some complicated comment explaining this. Better write
    if (temp.find("//")<temp.npos)
    to work correctly on all line lengths.

  • cout<<"The total number of comment lines is: "<<numbComments<<endl;
    Actually, you counted the number of end-of-line comments. I wouldn't call a comment at the end of a statement a "comment line".

  • You don't count /* */ style comments.

  • Counting the number of classes as };? Really? How about structs, enums, and plain superfluous semicolons? Simply count the number of occurences of the class keyword. It should have no alphanumeric character or underscore on either side.

Thomas
Counting the number of classes is a lot harder than that anyway. What if there's a string like "this contains the word class to fool your counting"?
IVlad
True. To do it properly, you'll have to ignore the word inside comments and strings. The preprocessor can still fool you, but when the preprocessor comes into play, there's no telling how many classes would actually exist unless you know the macros defined on the compiler command line, etcetera.
Thomas
Don't forget about `template<class T> ...`.
Bill
+2  A: 
  1. Use proper indentation, your code is very difficult to read in its current form. Here is a list of styles.
  2. Prefer ++variable instead of variable += 1 when possible; the ++ operator exists for a reason.
  3. Be consistent in your coding style. If you're going to leave spaces between things like cout and <<, function arguments and the function parantheses do it, otherwise don't, but be consistent. Pick one naming convention for your variables and stick to it. There is a lot about styles you can find on google, for example here and here.
  4. Don't use the entire std namespace, only what you need. User either using std::cout; or prefix all of your cout statements with std::
  5. Avoid needless comments. Everyone knows what ifstream infile(file_name.c_str()); does for example, what I don't know is what your program does as a whole, because I don't really care to understand what it does due to the indentation. It's a short program, so rather than explaning every statement on its own, why not explain what the program's purpose is, and how to use it?

These are all stylistic points. Your program doesn't work in its current form, assuming your goal is to count comments and classes. Doing that is a lot more difficult than you are considering. What if I have a "};" as part of a string for example? What if I have comments in strings?

IVlad
well, for my case it does work- seeing that it is for a project and i get the test file before i have to submit- so i can check for those sort of things- i realise it is not generic enough though - i thought there would be simple solutions of making it more so...
ace
+4  A: 

Format the code properly, use consistent style and nomenclature and throw out the utterly redundant comments and empty lines. The resulting code should be fine. Or “pro”.

Here, I’ve taken the efford (along with some stylistic things that are purely subjective):

Notice that the output is actually wrong (just run it on the program code itself to see that …).

#include <fstream>
#include <iostream>
#include <string>
#include <vector>

using namespace std;

int main()
{
    string file_name;

    cout << "please enter file name: ";
    cin  >> file_name;

    ifstream infile(file_name.c_str());

    if (not infile) {
        cerr << "Unable to open file " << file_name << " -- quitting!" << endl;
        return -1;
    }
    else cout << endl;

    vector<string> lines_of_text;
    string textline;

    while (getline(infile, textline)) {
        lines_of_text.push_back(textline);
        cout << lines_of_text.back() << endl;
    }

    cout << "OUTPUT BEGINS HERE:  " << endl << endl;
    cout << "the total capacity of vector: lines_of_text is: "
         << lines_of_text.capacity() << endl << endl;

    int ploc = lines_of_text.size() + 1;

    cout << "The total number of physical lines of code is: " << ploc << endl;

    // Look for comments `//` and count them.
    int num_comments = 0;
    for (vector<string>::iterator i = lines_of_text.begin();
            i != lines_of_text.end();
            ++i) {
        if (i->find("//") != string::npos)
            ++num_comments;
    }

    cout << "The total number of comment lines is: " << num_comments << endl;

    // Same for number of classes ...
}
Konrad Rudolph
awesome, that looks great
ace