Simple class exercise using the "this" pointer This is my solution for an exercise from a book.  It's a simple Golf class with member variables fullname and handicap. The constructor sets the variables to the provided name and handicap using values passed as arguments. The setgolf() function solicits name and handicap from user, creates a temporary Golf object and assigns it to the invoking object. 

Is this a proper solution? I'm particularly interested in the setgolf() method.

golf.h:

#include &lt;string&gt;

class Golf
{
    private:
        std::string fullname;
        int handicap;
    public:
        Golf();
        Golf(const std::string name, int hc);
        int setgolf();
        void sethandicap(int hc);
        void showgolf();
};


golf.cpp:

#include "golf.h"
#include &lt;cstring&gt;
#include &lt;iostream&gt;

Golf::Golf(const std::string name, int hc)
{
    fullname = name;
    handicap = hc;
}

int Golf::setgolf()
{
    std::string name;
    int hc;
    std::cout &lt;&lt; "Enter the name: ";
    getline(cin, name);
    if (name == "")
        return 0;
    std::cout &lt;&lt; "Enter the handicap: ";
    std::cin &gt;&gt; hc;
    *this = Golf(name, hc);
    return 1;
}
It's hard to say whether the solution is proper when there isn't a problem to solve.  How a class should be designed depends on how it will be used; a class in isolation is hard to judge.  Even if you want to focus on some specific aspect, try to fit it into the context of a larger problem.

Back to the code: I suspect showgolf should be a const function.  I don't see why printing a Golf would change it.  You probably also meant to pass your constructor a const std::string&amp;.  Finally, are you sure a showgolf member function is really what you want -- why not an operator&lt;&lt; overload?

Moving on to the Golf::setgolf function specifically, I don't think such a function is meaningful.  An operator&gt;&gt; overload or a free queryGolf function sound significantly more sensible to me.  More specifically, the usage:

Golf golf;
golf.setgolf();


Why not just auto golf = queryGolf();?  It's simpler, it means you can't forget to check whether things succeeded, and given setGolf's implementation it is likely to be just as efficient.

Speaking of checking whether things succeeded, why not throw an exception if the input is invalid?  There are valid reasons, but it's certainly a solution you should consider.  (By the way, if my previous solution sounded bad due to the need for exceptions, have no fear: boost::optional fits the use-case very well.)

And, in a similar vein: it sounds like you're trying to prevent the creation of invalid Golfs.  However, your default constructor does exactly that.  Are you sure default-construction is more valuable than guaranteed validity?  Again, depending on what you do with the class, the answer varies.

All in all, I'd suggest dropping showgolf and writing:

Golf promptGolf() {
    Golf golf;
    // read into fullname and handicap.  promptGolf may be a friend.
    if (/* golf is not valid, std::cin died, etc. */)
        throw TerribleError{};
    return golf;
}