get/set, struct, encapsulation ...

recently i've read an article that claims that every member w/ get/set method
is a quasi-class( badly designed )

My question is if get/set methods are bad, what if i have to modify the class's contents? something like the code below w/c represents an account w/ username & password, what should
i do instead ?

One answer i've search is to simply make it a data structure, a POD, no methods, etc..
Does this violate the rule of OOP and C++, encapsulation ?

what would you prefer, the first code, or the second one w/c is just a simple struct
1)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
class User {
public :
    User() {}
    User(
        const std::string username,
        const std::string password )
        : username_( username ), password_( password ) { }
    
    bool same( const User& user ) const {
        return( this->username_ == user.username_
                &&
                this->password_ == user.password_ );
    }
    void setUsername( const char* username ) { username_ = username; }
    void setPassword( const char* password ) { password_ = password; }
    std::string username() const { return username_; }
    std::string password() const { return password_; }
    
    inline friend std::ostream& operator << ( std::ostream& stream,
                                              const User& user );
    inline friend std::istream& operator >> ( std::istream& stream,
                                              User& user );
    inline friend bool operator== ( const User& left, const User& right );
private :
    std::string username_;
    std::string password_;
};


2)
1
2
3
4
5
6
7
struct User {
    std::string username;
    std::string password;
};

// and operate w/o using any member or overloaded operators and directly
// access the members 
 
My question is if get/set methods are bad, what if i have to modify the class's contents?  

Then create a set method.
Gets and Sets aren't bad as such, but having a get and set for every member in class, like you say in your first line, is the real issue in my opinion.
Last edited on
It's difficult to figure out what your real question is.

My question is if get/set methods are bad, what if i have to modify the class's contents?
Why should they be bad? They are good.

I think your real question is: Why the hell do I declare my class variables private and then I have to write extra setter and getter methods.. looks like just extra work.

Not every class variable is supposed to ever be changed outside the class.
By making them private you make sure that the "outside world" has no access and you can be sure while debugging that you haven't written somewhere in line 837 in your main() password = "this wasn't intended";

So next part: Now we want the "outside" world to access your variables. Why not now make them public?
Simple answer: Because you have better control about what's happening with your variables.
You can implement security checks. Throw an exception for example when your password is less than 6 characters. (which you should do ^.^)
Or checks that make sure that nothing crashes on runtime.
Last edited on
This really comes down to class design. You need to answer the question "What is the service my class is providing to other code?"

Once you've answered that question, that informs your decisions as to what the interface to the class should be. What information/data should the class be able to supply to its clients? What instructions should the clients be able to give to the class? In what ways should clients be able to change the state of the class?

Some of those things will map directly to getting/setting data members. Some won't. Sometimes, your interface will be allowing client code to get and set things that don't have a 1-to-1 relationship with data members. Most of the time, your class will have data members that don't have get/set methods, because they're only used by the class itself.

And even when your interface design does result in you having methods that directly retrieve/change the value of a data member, I'm firmly of the opinion that you should still encapsulate that data behind getters and setters. Because that gives you the freedom to change your class implementation if you want to, while still keeping the same interface, which means you don't have to go and change all the other code that uses your class.

(Or, at least, it keeps the amount of times you have to do that to a minimum. The reality is that there are plenty of times when you still want to change the interface to your class. But having a well-encapsulated interface means you only do it when it's really necessary.)
Thanks for the advises, it's great, now i have another question, which one do you prefer, integrate get/set into 1 function, like :
1
2
std::string& username() { return username_; }
std::string& password() { return password_; }

or a separate get/set for each members ?

And regarding Glandy's advice in throwing exceptions when, say the password entered is less than 6 chars, w/c do you prefer,
throw an exception, or simply return a value from the function indicating a failure and leaving internal data unchanged ?
1
2
3
4
5
6
7
8
9
10
11
const int SUCCESS_STATUS = 1;
const int FAIL_STATUS = 0;
// ...
int setUsername( const char* username ) {
    if( /* username is invalid */ ) {
        return FAIL_STATUS;
    }
    // ...

    return SUCCESS_STATUS
}

or
1
2
3
4
5
void setUsername( const char* username ) throw std::string {
    if( /* invalid */ ) {
        throw std::string( "Error" );
    }
}
which one do you prefer, integrate get/set into 1 function, or a separate get/set for each members ?

Separate. Read up on const correctness.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
#include <iostream>
#include <string>

class User
{
public:
    std::string& name() { return name_; }

private:
    std::string name_ ;
};

int main()
{
    const User user ;
    std::cout << user.name() << '\n' ;
}


http://ideone.com/cyDaPB
¿what about const correctness?
1
2
    std::string& name() { return name_; }
    const std::string& name() const { return name_; }
¿what about const correctness?


It keeps you from combining both into one function, as you just illustrated.

[Edit: And, of course, for the client to use the const correct version of the overloaded function when it wishes to ensure no changes to the object, it must cast the object to const before invoking the function, instead of just calling the correct function.]
Last edited on
thanks, how about in throwing exceptions w/c one do you guys prefer,
throw an exception, or simply return a value from the function indicating a failure and leaving internal data unchanged ?
1
2
3
4
5
6
7
8
9
10
11
const int SUCCESS_STATUS = 1;
const int FAIL_STATUS = 0;
// ...
int setUsername( const char* username ) {
    if( /* username is invalid */ ) {
        return FAIL_STATUS;
    }
    // ...

    return SUCCESS_STATUS
}

or
1
2
3
4
5
void setUsername( const char* username ) throw std::string {
    if( /* invalid */ ) {
        throw std::string( "Error" );
    }
}


since the try catch syntax is a little long compared to a simple if ( or while )
1
2
3
4
5
try {
    user1.setUsername( "blah" );
} catch( ... ) {
    // ... handle
}

compared to:
1
2
3
if( !user1.setPassword( "blah" ) ) {
    // ...
}
Last edited on
BUMP
Last edited on
BUMP again
Validation belongs in the code that gets the input from the user, so "neither" would be my answer.
I think the object is more qualified to validate the data.
By instance, a file object that verifies that the path string actually refers to a disk file and that you do have permissions to operate on it.
Topic archived. No new replies allowed.