first program evaluation

P

Pieter Provoost

Hi,

I just made my first c++ program, and I would like to know if there are
things I can do better. Also, the exe is about 500 kb, can I make it
smaller?

Thanks!
Pieter




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

int main()
{
string buffer;
string includebuffer;
string bestand;
string configin;
string configout;

ifstream config("config.txt");
getline(config, configin);
getline(config, configout);
config.close();

ifstream infile(configin.c_str());
ofstream outfile(configout.c_str());

while (getline(infile, buffer))
{
if (buffer == "")
{
outfile << " " << endl;
}
else
{
if (buffer.substr(1,7) == "include")
{
bestand = buffer.substr(9, (buffer.find_first_of("}") - 9));
bestand += ".tex";
ifstream includefile(bestand.c_str());
while (getline(includefile, includebuffer))
{
outfile << includebuffer << endl;
}
includefile.close();
}
else
{
outfile << buffer << endl;
}
}
}

infile.close();
outfile.close();
return 0;
}
 
P

Phlip

Pieter said:
I just made my first c++ program, and I would like to know if there are
things I can do better.

Write unit tests for every function.
Also, the exe is about 500 kb, can I make it
smaller?

That is a FAQ for this newsgroup. The good news is as your program gets
larger the executable size will not grow proportionally. You can add a lot
of code before you hit 501 kb. The executable is big because many features
you did not use got linked in.
#include <string>
#include <fstream>
using namespace std;

Don't use 'using namespace'. Invite only the identifiers you need in, such
as using std::string. As a program gets huge, many 'using namespace'
statements will cause major problems, including bugs in your compiler, and
name collisions.
int main()
{
string buffer;
string includebuffer;
string bestand;
string configin;
string configout;

Put each of these as close as possible to the code that uses them.
ifstream config("config.txt");
getline(config, configin);
getline(config, configout);
config.close();

Cut these up into functions. But...
ifstream infile(configin.c_str());
ofstream outfile(configout.c_str());

....these could have been command line arguments. Research argv and argc.
while (getline(infile, buffer))

Nice. Some folks at this point use either infile >> string, or scanf. Both
are user hostile.
{
if (buffer == "")
{
outfile << " " << endl;

Why the little space?
}
else
{
if (buffer.substr(1,7) == "include")
{
bestand = buffer.substr(9, (buffer.find_first_of("}") - 9));

That line does too much. Pull out the first find_first_of, and put its
result in a variable, and give that variable a name.
bestand += ".tex";
ifstream includefile(bestand.c_str());
while (getline(includefile, includebuffer))
{
outfile << includebuffer << endl;
}
includefile.close();
}
else
{
outfile << buffer << endl;

If you did not have the if(buffer == "") up there, then control flow would
drop to here, and do the same thing.
}
}
}

infile.close();
outfile.close();
return 0;
}

Good luck!
 
P

Pieter Provoost

Phlip said:
Write unit tests for every function.


That is a FAQ for this newsgroup. The good news is as your program gets
larger the executable size will not grow proportionally. You can add a lot
of code before you hit 501 kb. The executable is big because many features
you did not use got linked in.


Don't use 'using namespace'. Invite only the identifiers you need in, such
as using std::string. As a program gets huge, many 'using namespace'
statements will cause major problems, including bugs in your compiler, and
name collisions.

Ok, I'll have to look that up (don't know how to use it).

Put each of these as close as possible to the code that uses them.


Cut these up into functions. But...


...these could have been command line arguments. Research argv and argc.

They were command line arguments at first, but I prefer to work with the
config file and jut click the icon. I stopped using the dos command prompt
since they introduced directories with names such as "documents and
settings"...

Nice. Some folks at this point use either infile >> string, or scanf. Both
are user hostile.


Why the little space?

Well, when I didn't have this line, I used to get an error when the program
tried to process a blank line. Maybe it's because I can't apply substring to
an empty string?

That line does too much. Pull out the first find_first_of, and put its
result in a variable, and give that variable a name.


If you did not have the if(buffer == "") up there, then control flow would
drop to here, and do the same thing.

See my comments above.

Good luck!


Thank you very much!
Pieter
 
P

Pieter Provoost

Pieter Provoost said:
Ok, I'll have to look that up (don't know how to use it).

Found it. This does the job:

using std::string;
using std::endl;
using std::ifstream;
using std::eek:fstream;
using std::getline;
 
B

Bruce

In comp.lang.c++
Phlip said:
Write unit tests for every function.
Opinion


Don't use 'using namespace'. Invite only the identifiers you need in, such
as using std::string. As a program gets huge, many 'using namespace'
statements will cause major problems, including bugs in your compiler, and
name collisions.
Opinion


Put each of these as close as possible to the code that uses them.
Opinion


Cut these up into functions. But...
Opinion


...these could have been command line arguments. Research argv and argc.
Opinion


That line does too much. Pull out the first find_first_of, and put its
result in a variable, and give that variable a name.

Opinion
 
G

Gregg

Found it. This does the job:

using std::string;
using std::endl;
using std::ifstream;
using std::eek:fstream;
using std::getline;

I think you will find it to be tedious having to add a "using" statement
everytime you decide to use something from the standard library. I find it
much easier to attach "std::" in front of standard library types whenever I
use them.

std::cout << "Hope this helps" << std::endl;

Gregg
 
E

E. Robert Tisdale

Pieter said:
I just made my first c++ program,
and I would like to know if there are things I can do better.
Also, the exe is about 500 kb, can I make it smaller?
cat main.cc
#include <string>
#include <cstdlib>
#include <fstream>
#include <iostream>

int main(int argc, char* argv[]) {
using namespace std;
int result = EXIT_SUCCESS;

ifstream config("config.txt");
if (config) {
string configin;
if (getline(config, configin)) {
ifstream infile(configin.c_str());
if (infile) {
string configout;
if (getline(config, configout)) {
ofstream outfile(configout.c_str());
if (outfile) {
string buffer;
string includebuffer;
while (getline(infile, buffer)) {
if (buffer == "") {
outfile << " " << endl;
}
else { // (buffer != "")
if (buffer.substr(1,7) == "include") {
string bestand = buffer.substr(
9, (buffer.find_first_of("}") - 9));
bestand += ".tex";
ifstream includefile(bestand.c_str());
if (includefile) {
while (getline(includefile, includebuffer)) {
outfile << includebuffer << endl;
}
includefile.close();
}
else {// (!includefile)
cerr << "Couldn't open include file: "
<< bestand << endl;
result = EXIT_FAILURE;
break;
}
}
else { // (buffer.substr(1,7) != "include")
outfile << buffer << endl;
}
}
}
outfile.close();
}
else { // (!outfile)
cerr << "Couldn't open output file: "
<< configout << endl;
result = EXIT_FAILURE;
}
}
else { // (!getline(config, configout))
cerr << "Couldn't read output file name!" << endl;
result = EXIT_FAILURE;
}
infile.close();
}
else { // (!infile))
cerr << "Couldn't open input file: "
<< configin << endl;
result = EXIT_FAILURE;
}
}
else { // (!getline(config, configin))
cerr << "Couldn't read input file name!" << endl;
result = EXIT_FAILURE;
}
config.close();
}
else { // (!config)
cerr << "Couldn't open file: config.txt" << endl;
result = EXIT_FAILURE;
}
return result;
}
g++ -Wall -ansi -pedantic -o main main.cc
ls -l main
-rwxr-xr-x 1 edwin jpl 12188 May 31 13:48 main
 
D

Daniel T.

Pieter Provoost said:
I just made my first c++ program, and I would like to know if there are
things I can do better. Also, the exe is about 500 kb, can I make it
smaller?

Is there a particular reason you are worried about the size of the exe?

I want to try the Socratic method on you:
bestand = buffer.substr(9, (buffer.find_first_of("}") - 9));
bestand += ".tex";
ifstream includefile(bestand.c_str());
while (getline(includefile, includebuffer))
{
outfile << includebuffer << endl;
}
includefile.close();

What is the code above supposed to do? Use standard English to describe
it...
 
D

David Harmon

On Mon, 31 May 2004 13:50:19 -0700 in comp.lang.c++, "E. Robert Tisdale"
else { // (!outfile)
cerr << "Couldn't open output file: "
<< configout << endl;
result = EXIT_FAILURE;
}
}
else { // (!getline(config, configout))
cerr << "Couldn't read output file name!" << endl;
result = EXIT_FAILURE;
}
infile.close();
}
else { // (!infile))
cerr << "Couldn't open input file: "
<< configin << endl;
result = EXIT_FAILURE;
}
}
else { // (!getline(config, configin))
cerr << "Couldn't read input file name!" << endl;
result = EXIT_FAILURE;
}
config.close();
}
else { // (!config)
cerr << "Couldn't open file: config.txt" << endl;
result = EXIT_FAILURE;
}

This is *very* bad advice.
 
P

Pieter Provoost

Daniel T. said:
Is there a particular reason you are worried about the size of the exe?

I want to try the Socratic method on you:


What is the code above supposed to do? Use standard English to describe
it...

It's supposed to replace the line "\include{somefile.tex}" in a LaTeX file
by the contents of somefile.tex. This way I have the complete source of my
document in one file.
 
P

Pieter Provoost

Daniel T. said:
Is there a particular reason you are worried about the size of the exe?

I want to try the Socratic method on you:


What is the code above supposed to do? Use standard English to describe
it...

About the file size: no there's no particular reasin, but I just supposed it
would be a lot smaller since it's quite simple.
 
K

Karl Heinz Buchegger

Pieter said:
About the file size: no there's no particular reasin, but I just supposed it
would be a lot smaller since it's quite simple.

Don't know if somebody already asked this:
Are you sure you have turned on compiler optimization and
are not building a 'debug' version?

Debug versions usually are much larger then release version,
since a lot of symbol table information is packed into the
executable among other things.
 
P

Pieter Provoost

Karl Heinz Buchegger said:
Don't know if somebody already asked this:
Are you sure you have turned on compiler optimization and
are not building a 'debug' version?

Debug versions usually are much larger then release version,
since a lot of symbol table information is packed into the
executable among other things.


Ah I probably made a debug version. I used msdev and the exe ended up in a
directory called "debug". I didn't know optimization is possible, I'll check
it out right away.

Thanks!
 
P

Pieter Provoost

Ah I probably made a debug version. I used msdev and the exe ended up in a
directory called "debug". I didn't know optimization is possible, I'll check
it out right away.

Thanks!

Can anybody help me here? I can't find it in VC++.
 
H

Howard

David Harmon said:
On Mon, 31 May 2004 13:50:19 -0700 in comp.lang.c++, "E. Robert Tisdale"


This is *very* bad advice.

Care to elaborate? What's the "very bad" part? Using such deep nesting?
Using cerr? Using failure result codes? Or...?

For maximum protection, I'd utilize RAII for handling resources (and the
success or failure of obtaining them). But given the simpicity of the
original post, I'd suggest that might simply confuse the user with a lot of
overhead. They can get into RAII later.

But I'm curious as to exactly what you've found in his response that was
"very bad"...? (Given a cursory glance, it looks like somewhat typical
open/close nesting code.)

-Howard
 
P

Pieter Provoost

Howard said:
Care to elaborate? What's the "very bad" part? Using such deep nesting?
Using cerr? Using failure result codes? Or...?

For maximum protection, I'd utilize RAII for handling resources (and the
success or failure of obtaining them). But given the simpicity of the
original post, I'd suggest that might simply confuse the user with a lot of
overhead. They can get into RAII later.

Exactly!

But I'm curious as to exactly what you've found in his response that was
"very bad"...? (Given a cursory glance, it looks like somewhat typical
open/close nesting code.)

-Howard
 
K

Karl Heinz Buchegger

Pieter said:
Can anybody help me here? I can't find it in VC++.

First of all: This isn't really a question on C++ but on the
tool you use. Thus it should be asked in a hewsgroup dedictaed
to VC++. Look at Shivas Welcome message, it contains some suggestions
of where to ask. http://www.slack.net/~shiva/welcome.txt

Anyway: 2 Methods

1. You can select the 'active configuration' with
Menu: 'Build' 'Set active configuration'

2. Start VC++, Right click in the toolbar section (anywhere in empty space).
Select 'Customize'. A dialog opens, make sure you have the 'Commands' tab
selected. Select the 'Build' category. In the right part of the dialog some
buttons and a combo box appear. Klick that combo box and drag it into a
toolbar. Click 'Close'

In this combo box, the 'active configuration' (Debug - Release) will be shown and
can be changed.
 
D

Daniel T.

Pieter Provoost said:
It's supposed to replace the line "\include{somefile.tex}" in a LaTeX file
by the contents of somefile.tex. This way I have the complete source of my
document in one file.

OK then, lets turn that into a function:

void insertFile( ostream& os, const string& fileName )
{
ifstream includefile( fileName );
string includebuffer;
while ( getline( includefile, includebuffer ) )
os << includebuffer << endl;
}

In main we have:

string bestand = buffer.substr(9, (buffer.find_first_of("}") - 9));
bestand += ".tex";
insertFile( outfile, bestand );

and we can remove the line:

string bestand;

From the top of the file.

What do you think of this minor change? Does it make main easer to
understand or harder? Is the function created easy to understand? Do you
think you could use the insertFile function in other contexts?
 
P

Pieter Provoost

Karl Heinz Buchegger said:
First of all: This isn't really a question on C++ but on the
tool you use. Thus it should be asked in a hewsgroup dedictaed
to VC++. Look at Shivas Welcome message, it contains some suggestions
of where to ask. http://www.slack.net/~shiva/welcome.txt

Anyway: 2 Methods

1. You can select the 'active configuration' with
Menu: 'Build' 'Set active configuration'

2. Start VC++, Right click in the toolbar section (anywhere in empty space).
Select 'Customize'. A dialog opens, make sure you have the 'Commands' tab
selected. Select the 'Build' category. In the right part of the dialog some
buttons and a combo box appear. Klick that combo box and drag it into a
toolbar. Click 'Close'

In this combo box, the 'active configuration' (Debug - Release) will be shown and
can be changed.

Thanks! It's about 140 kb now instead of 500.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,580
Members
45,055
Latest member
SlimSparkKetoACVReview

Latest Threads

Top