Class design question.

[Jason Heyes]

Is this class evidence of poor design?

class Rectangle
{
double width, height;
public:
double get_width() const { return width; }
double get_height() const { return height; }

void set_width(doubl e width) { this->width = width; }
void set_height(doub le height) { this->height = height; }
};

If so then why?

[Alf P. Steinbach]

* Jason Heyes:

Is this class evidence of poor design?

class Rectangle
{
double width, height;
public:
double get_width() const { return width; }
double get_height() const { return height; }

void set_width(doubl e width) { this->width = width; }
void set_height(doub le height) { this->height = height; }
};
Yes.

If so then why?

No user defined constructor => undefined state after construction.

And as it stands (it would be different with a constructor) no way
to assign both width and height at the same time.

Also there is evidence of poor coding, namely inconsistent use of
'this->'; even using it at all in C++ is generally poor style, but
using it inconsistently, just as a disambiguator, is worse.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

[Ivan Vecerina]

"Jason Heyes" <ja********@opt usnet.com.au> wrote in message
news:41******** **************@ news.optusnet.c om.au...

Is this class evidence of poor design?

class Rectangle
{
double width, height;
public:
double get_width() const { return width; }
double get_height() const { return height; }

void set_width(doubl e width) { this->width = width; }
void set_height(doub le height) { this->height = height; }
};

If so then why?

If there are no class invariants to enforce, and no reason
to expect having to add this capability in the future, I
would consider using a plain struct instead:
struct Rectangle {
//consider adding a constructor...
double width, height;
};
--
http://ivan.vecerina.com/contact/?subject=NG_POST <- email contact form

[osmium]

"Jason Heyes" writes:

Is this class evidence of poor design?

class Rectangle
{
double width, height;
public:
double get_width() const { return width; }
double get_height() const { return height; }

void set_width(doubl e width) { this->width = width; }
void set_height(doub le height) { this->height = height; }
};

If so then why?

Yes, it's poor design. It is just an obfuscated way of doing what could be
done with a simple struct. There should be some reward for complexity,
there are no rewards here.

[Matthias Käppler]

Jason Heyes wrote:

Is this class evidence of poor design?

class Rectangle
{
double width, height;
public:
double get_width() const { return width; }
double get_height() const { return height; }

void set_width(doubl e width) { this->width = width; }
void set_height(doub le height) { this->height = height; }
};

If so then why?

Poor design in terms of what? Notation? Mechanics?

Some might argue that since you are giving read -and- write access to width
and height, you could as well make width and height public (like someone
else already mentioned).

However, I think in terms of OOP your approach is better, because you are
e.g. free to change the names of your class members at any time without
affecting the actual use your class for a potential client.

As to the style thing, I'd prefer notating data members with m_* rather than
using 'this' to express membership, but that's just a matter of taste I
guess.

[Matthias Käppler]

osmium wrote:

Yes, it's poor design. It is just an obfuscated way of doing what could
be
done with a simple struct. There should be some reward for complexity,
there are no rewards here.

If the class will never change, then you might be right.

Otherwise this is very short thought.
If for example some day you happen to want the class doing more tha just
setting the width or height, like e.g. logging the action, then in your
approach you would have to completely redesign your class. The client would
have to change ALL his calls where he is settings width and height.

That can't happen with his approach, thus, in my eyes, it's by far better
style than making it a struct (which I try to avoid in general in C++,
since it's more a backward compatibility to C thing instead of a useful C++
construct).

[osmium]

"Matthias Käppler" writes:

osmium wrote:

Yes, it's poor design. It is just an obfuscated way of doing what could
be
done with a simple struct. There should be some reward for complexity,
there are no rewards here.

If the class will never change, then you might be right.

Otherwise this is very short thought.
If for example some day you happen to want the class doing more tha just
setting the width or height, like e.g. logging the action, then in your
approach you would have to completely redesign your class. The client
would
have to change ALL his calls where he is settings width and height.

That can't happen with his approach, thus, in my eyes, it's by far better
style than making it a struct (which I try to avoid in general in C++,
since it's more a backward compatibility to C thing instead of a useful
C++
construct).

I think the notion of writing for the future is mostly bullshit. The idea
that people can realistically predict the future is bogus. This is an
example of OOP gone wild. One of the first steps in design is to decide
what you want to design. An airplane that characteristica lly does a
controlled crash landing on an aircraft carrier should have a different
design than an
airplane that lands on a runway. A composite design penalizes both
applications.

There is entirely too much hokum involving OOP, it is not a silver bullet
and the sooner people realize that the better.

[Matthias Käppler]

osmium wrote:

I think the notion of writing for the future is mostly bullshit. The idea
that people can realistically predict the future is bogus.
Exactly, that's what I'm talking about. And that's why it's often a good
idea to design your class loose so you can extend it at any time without
changing its outer appearance.
This is an
example of OOP gone wild. One of the first steps in design is to decide
what you want to design.
If your designs hold 100% from day one to release, than you're an extremely
good software engineer :)
Most designs do not, that's why it's a good idea to keep some doors open.
There is entirely too much hokum involving OOP, it is not a silver bullet
and the sooner people realize that the better.

Opinions may differ here I guess.

Cheers,
Matthias

[Gary Labowitz]

"osmium" <r1********@com cast.net> wrote in message
news:30******** *****@uni-berlin.de...

"Matthias Käppler" writes:

osmium wrote: I think the notion of writing for the future is mostly bullshit. The idea
that people can realistically predict the future is bogus. This is an
example of OOP gone wild. One of the first steps in design is to decide
what you want to design. An airplane that characteristica lly does a
controlled crash landing on an aircraft carrier should have a different
design than an
airplane that lands on a runway. A composite design penalizes both
applications.

But designing to allow for ease of modification is the point, not knowing
the future.

There is entirely too much hokum involving OOP, it is not a silver bullet
and the sooner people realize that the better.

Amen to that, brother.
--
Gary

[Gianni Mariani]

osmium wrote:
....

I think the notion of writing for the future is mostly bullshit. The idea
that people can realistically predict the future is bogus. This is an
example of OOP gone wild. One of the first steps in design is to decide
what you want to design. An airplane that characteristica lly does a
controlled crash landing on an aircraft carrier should have a different
design than an
airplane that lands on a runway. A composite design penalizes both
applications.

There is entirely too much hokum involving OOP, it is not a silver bullet
and the sooner people realize that the better.

There is a balance between overdoing it and underdoing it.

The way I express it is:
"Never artificially limit the utility of a class or function"

In other words, don't go out of your way to make somthing less generic.

As for the OP. I have no idea what the application of the class is so
it might be just perfect, it might not, I don't know.

If I needed a generic shape library, it's not even close.