Need some help readjusting program

[i8bjz8AR]

Original:Write a program that asks the user for the name of a file. The program should display
the first 10 lines of the file on the screen (the “head” of the file). If the file has fewer than 10 lines, the entire file should be displayed, with a message indicating the entire
file has been displayed.

Hi guys, I wrote a program to ask a user for a file and if the file exists + has less then 10 lines, then it will show all the text. However, if file has more then 10 lines, then only display the first 10. Could somebody run through my code and check to see if this is correct? My file, "nameFile.txt" has 13 lines and all 13 are showing in my program, as I think this is wrong. Could somebody help me fix this?

#include <iostream>
#include <fstream>
#include <string>
#include <cstdlib>
using namespace std;

int main() {
cout << "This program will display the first 10 lines of a program.\n";
cout << "Please enter the name of the file: ";
string nameFile;
getline(cin, nameFile);

fstream file(nameFile, ios::in);

if(file.fail())
{
cout << "The input file could not be found!\n";
return 0;
}

string line;
int counter = 0;
while(getline(file, line))
{
++counter;
}
file.close();

file.open(nameFile, ios::in);

if(counter < 10)
{
cout << "\nThe file has fewer than 10 lines, the entire file will now be displayed below:\n\n";
while(getline(file, line))
{
cout << line << endl;
}
}
else
{
counter = 1;
cout << "\nThe file has more than 10 lines. Here are the first 10 lines\n";

while(getline(file, line))
{
if(counter <= 10)
{
cout << line << endl;
}
else if(counter > 10)
{
break;
}
}
}
}

[TheIdeasMan]

Hi,

You are missing the increment of counter near line 48.

Also, if one knows how many times something needs to happen, then use a for loop. Use a const unsigned variable to control the number of times it loops.

To be clever, you can check on the state of file.eof at the end of the for loop, to see if it has reached the end or not, and take appropriate action either way. Note, don't loop on eof, it's bad practise.

This might cut down on the size of the code, and make it a little more elegant.

Now style wise, your code needs some indentation, if using an IDE this should be automatic. If it's after the fact, you should be able to find an option somewhere to re-format all the code.

Avoid having line 5, it defeats the purpose of namespaces, because it can cause name conflicts. Put std:: before each std thing - believe me it's the best way - all the expert coders do this. Have a read on Google about namespaces.

The other good bit of advice is about the use of functions, algorithms, classes etc anywhere - not just in the STL, is to always read the documentation for the thing you want to use. There is reference material and tutorials at the top left of this page. If you want very technical and comprehensive info about C++ standard compliant features then there is cppreference.com. There is also stackoverflow.com to search -someone has bound to have already had your problem. :+)

Regards

[i8bjz8AR]

Thanks for the response! Also very interesting about the namespace std concept, I know everybody in my class (Computer Science 1) probably does it in that way so I'll definitely check it out.

On line 48 then, I'm confused on what I need to add. Do i just need to add counter++?

[TheIdeasMan]

On line 48 then, I'm confused on what I need to add. Do i just need to add counter++?

Yes. At the moment you are looping, but the test on line 46 has no effect because you don't change counter.

Also very interesting about the namespace std concept, I know everybody in my class (Computer Science 1) probably does it in that way so I'll definitely check it out.

I don't know why all the book authors (including Stroustrup) , Lecturers, web sites, do this. It seems easier, but really it's bad. If you change to doing std:: , you will be ahead of the eight ball, just as long as you can explain why, to anyone who asks :+)

The big thing about this is that there are a number of things in std that one might easily try to use as your own variable or function name, such as left, right, distance, count quote plus heaps more.

Ideally you own code should be in it's own namespace.

In some of my code, I have a namespace GDA, so when referring to anything in it I prefix with GDA:: that way if there were others working on that project with me, they can use the same names as me and there wouldn't be any conflict, as long as they use different namespaces to me. Anyone working in GDA has to respect the names already there. This is really important for large code bases, one might as well get into the habit for small amounts of code too.

Regards :+)

[pearlyman]

@ Ideasman

I like that concept of GDA:: That's cool.

I would like to see an example of this, I'm curious... You may have started something. :)

[i8bjz8AR]

okay i added the counter++, along with some curly braces but now I'm getting a "else" without a previous "if" error.. I'm very new to looping, what do I need to add to build successfully?

#include <iostream>
#include <fstream>
#include <string>
#include <cstdlib>
using namespace std;

int main() {
cout << "This program will display the first 10 lines of a program.\n";
cout << "Please enter the name of the file: ";
string nameFile;
getline(cin, nameFile);

fstream file(nameFile, ios::in);

if(file.fail())
{
cout << "The input file could not be found!\n";
return 0;
}

string line;
int counter = 0;
while(getline(file, line))
{
++counter;
}
file.close();

file.open(nameFile, ios::in);

if(counter < 10)
{
cout << "\nThe file has fewer than 10 lines, the entire file will now be displayed below:\n\n";
while(getline(file, line))
{
cout << line << endl;
}
}
else
{
counter = 1;
cout << "\nThe file has more than 10 lines. Here are the first 10 lines\n";

while(getline(file, line))
{
if(counter <= 10){
    counter++;
}
{
cout << line << endl;
}
else if(counter > 10)
{
break;
}
}
}
}

[TheIdeasMan]

while(getline(file, line))
{
  cout << line << "\n";  // avoid endl can be inefficient, it flushes the buffer 
  counter++;

  if(counter > 10)
  {
     break;
  }
}

I wonder if you can figure out what I was saying earlier - by testing the file stream you can make the code much shorter and more elegant. Here is some pseudo code:

// while loop on state of file stream
  // print a line 
  // increment line counter

  // if line counter == 10
      // stop printing lines
      // carry on with counting
// end while loop

// print number of lines in file  

Something like that could be your whole program, note it goes through the file once.

[TheIdeasMan]

I should have mentioned magic numbers like 10 in your code, don't have them, make a const variable and use that throughout the code.

const unsigned short MaxLines = 10;

if(counter <= MaxLines) {

This goes back to what I said in my reply to your other topic - learn the capabilities of all the different types. This is an example of where unsigned is a good thing. short isn't necessary or quicker, but now you know it exists.

const is a very good thing in c++, use it where ever you can. For now function parameters should often be const, because you often don't want to change them inside the function. Further on, you will learn lots more uses for it.

[i8bjz8AR]

thanks so much for all the help! added those few lines and I'm seeing the output I want. Then making a const variable instead of a int would avoid something like junk data with a longer program? or would just be better in general?

[pearlyman]

defining it at creation would avoid junk data...

int a=whatever; is always better than int a=I don't know;

[TheIdeasMan]

Then making a const variable instead of a int would avoid something like junk data with a longer program? or would just be better in general?

Not sure if you were talking about avoiding the magic numbers, or const in general.

One advantages of having a const variable like MaxLines is that if you want to change that hard coded value, you can do so in one place in the file, not here there and everywhere.

The const concept is a thing that not so many programming languages have, and it is a huge bonus to have it. Basically, it allows the compiler to enforce the idea that a value shouldn't change, and that is very useful.

Consider this function:

constexpr double PI = 3.14159265358979323846;

double CircleArea (const double radius) {
   return radius * radius * PI;
}

The first thing is obvious, we don't want to be changing the value of PI. This is another usage of a const concept, the const expression qualifier constexpr - a c++11 standard thing - one needs to compile with c++11 to use this. It is more set in stone than const because const can be cast away (removed), whereas constexpr is forever const. constexpr can be applied to entire expressions and even functions. The whole thing must evaluate to one value (number, string whatever) at compile time.

The second one is the parameter to the function - radius. By making it const we are saying that the value of it cannot be changed inside the function, which is natural and desirable.

Most of the time function parameters should be const , an exception to this is parameters passed by reference so their value is changed in the outer scope:

constexpr double PI = 3.14159265358979323846;

void CircleData (const double radius, 
                       double& Area, 
                       double& Circumference) 
{
   Area =  radius * radius * PI;            // the value of these variables are changed
   Circumference = 2.0 * radius * PI;   // in the scope which called this function
}

Ok, so how did you get on with my suggestion for doing 1 pass through the file?

Actually it's not that different from the snippet I gave you. The if condition should be at the start of the while loop, put getline inside the while loop, go from there.

I reckon you could do the whole program in 15 lines.

About pseudo code: It might seem as boring as hell - one wants to get in there a start coding flat-out right? But it is actually a very good tool for organising ones thoughts, writing down the algorithm, identifying the need for functions and loops, help in seeing where improvements can be made. The extra work is worth it - one might have a 15 line program instead of a 50 line program.

The trick is to start out very simple and general as you like. So I might have had just this:

// Print 10 lines of a file

Then, go back and add more detail and refinement until you are happy to convert it to code. Leave the comments in, if you like - they can be a form of documentation.

// const unsigned short MaxLines = 10;
// Print Maxlines lines of a file
// Print how many lines in the file 

Then, more refinement:

// const unsigned short MaxLines = 10;
// Print Maxlines lines of a file
   // open the file
       // variable for file name std::string FileName
   // Print the lines
      // while loop condition  <--- not sure what the condition is yet, decide later if you want
      // end while loop
   // close the file
// Print how many lines in the file
   // variable LineCount  <--- decide the type later if you want 

So you keep going until you are happy you have enough detail.

In terms of seeing where improvements can be made, try to identify if anything is being done multiple times. This might mean a different algorithm or maybe slightly changing a function to cope with slightly different situations.

There you go, have a larrap at that :+)