"Hello, world!" program using a class for printing

Question

Please take a look at my program and let me know how I can improve it.

/*
"   To Print A Line On The Display Screen"
    Date:5th January 2011
    Programmer:Fahad
*/
#include <iostream>
using namespace std;
class Print
{
    public:
        void print_();
};
int main()
{
    Print Obj;
    Obj.print_();
    system( "pause" );
    return 0;
}
void Print::print_()
{
    cout << "I am in print function and the program runs fine." << endl;
}

Answer 1

In addition to the other comments, I would also use a different naming convention for types and objects.

For example, this looks unconventional.

Print Obj;
Obj.print_();

I prefer:

Print obj;
obj.print();

It's just a convention but being able to easily spot names that denote types helps if you start to use more complex expressions. For example:

Print().print();

Personally, I would also avoid system("pause"). You need to #include either <stdlib.h> or <cstdlib> to use it. Although the system call itself is standard C++ (from the standard C library), what you pass to it is system dependent.

In general I don't believe you should make your programs stop artificially. If they are designed to run in a terminal then the terminal user will be able to see the output even after the program exits.

Answer 2

In addition to what Billy said, I find Obj.print_() to look strange in C++ code. I would have just made the method name print().

Answer 3

Not sure what you mean by coding style. If you're talking about spacing and such, here are changes I would make. Note that these are entirely subjective and people are probably going to disagree with me.

Don't indent the public: specifier in the class -- leave it flush with the curly braces that mark the class definition. The reason for this is the implicit private region in the class.

Example code:

class Example
{
    int a; //Shouldn't this line
    public:
        void MyFunc(); //Indent to the same place this one does?
};


class Ahhhhh
{
    int a;  //Ahhh.. we match now :)
public:
    void MyFunc();
};

For that matter this class has no private members so I would just change class to struct and remove the access specifier entirely.

I would remove using namespace std; and explicitly qualify those members which are in std. Would really stink to get a nasty error message from the compiler because you happened to define a function called copy (which might conflict with std::copy).

system("pause"); should be std::cin.get();

If you're talking about "design", it seems overengineered to me. No reason to involve objects in a program like this at all. Just sticking the print statement in main would suffice.

Answer 4

The one thing that is not C++ like for me is this:

Obj.print_();

This tightly couples the print method to a particular output method.
It would be better to allow the user of your object to define what the output method is:

std::cout << Obj << "\n";

Which means you need to define an output operator for you object:

std::ostream& operator<<(std::ostream& str, Print const& data)
{
    // STUFF
    return str;
}

Though not technically wrong. I am not a fan of underscore at the ends of identifiers:

print_()

Looks wierd to me. But this is a style thing. See you local coding conventions for rules. If you had put it on the front I would have been a lot more complainey about it.

System is hard to use cross platform. Especially when you do system("pause").

system( "pause" );

I prefer the platform neutral:

std::cout << "Hit Enter to continue\n";
std::cin.clear();
char plop;
std::cin >> plop;  // cin is buffered. So nothing is sent until you hit enter.

Answer 5

Under the circumstances, a using directive seems highly suspect. While there are times/places that it's useful, this doesn't seem (to me) to be one of them.

The name of a typical class should also be a noun, not a verb. A verb signals that what you have is a single action, which is not a good candidate for a normal class. If it's going to be a class at all, it should probably be a functor. I'd also add a parameter (with a default value) so it would be easy to use a stream other than std::cout when/if necessary:

struct Print { 
    std::ostream &operator()(std::ostream &os = std::cout) { 
        return os << "whatever\n";
    }
};

Using system("pause"); is also quite non-portable. If you want to wait for the user to press a key before ending the program, it's generally better to build that into your own code:

void pause() { 
    std::cout << "Press \"enter\" when ready.\n";
    getchar();
}

int main() { 
    Print()();
    pause();
    return 0;
}

Frankly, even using the functor strikes me as silly in this case though -- you're taking something simple (print out a string) and making it much more complex without getting anything in return. Given how little the program does, the Print class accomplishes nothing useful or positive at all.

Answer 6

I prefer seeing class names that are nouns and method names that are verbs. Print might read better as Printer, ObjectPrinter or WhateverPrinter.

I've seen the _ suffix (or a m_ prefix) to denote members quite a bit but I've never found it useful to attach this sort of decoration to a name.

Answer 7

I'm really not a fan of putting the main() function in the middle. I prefer it to be the first thing in the file, or the last thing in the file. I would personally have defined your Print class in a header file, put the actual code for it a separate source file, and then put your main function in the main source file... This would make your Print class a lot easier to reuse in another application or turn into a library.

For example

Print.h

#ifndef __PRINT_H__
#define __PRINT_H__

class Print
{
    public:
        void print_();
};

#endif //__PRINT_H__

Print.cpp

#include <iostream>
#include "Print.h"

void Print::print_()
{
    std::cout << "I am in print function and the program runs fine." << std::endl;
}

main.cpp

/*
"   To Print A Line On The Display Screen"
    Date:5th January 2011
    Programmer:Fahad
*/
#include <stdlib.h>
#include "Print.h"

int main()
{
    Print Obj;
    Obj.print_();
    system( "pause" );
    return 0;
}

Answer 8

The concept of printing is better defined as a method of an object, for instance of the World class. For example, we can define the following:

#include <iostream>
int main(int argc, char* argv[]){
    class World{
    public:
        World(void){
            std::cout << "Hello " << __FUNCTION__ << "\n";
        }
    } Hello;
}

The constructor will print the message as soon as the World object is instantiated as Hello. It is more common to define the class outside of the main, but not forbidden...