473,473 Members | 2,031 Online
Bytes | Software Development & Data Engineering Community
Create Post

Home Posts Topics Members FAQ

Class design question.

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(double width) { this->width = width; }
void set_height(double height) { this->height = height; }
};

If so then why?
Jul 22 '05 #1
18 1404
* 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(double width) { this->width = width; }
void set_height(double 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?
Jul 22 '05 #2
"Jason Heyes" <ja********@optusnet.com.au> wrote in message
news:41**********************@news.optusnet.com.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(double width) { this->width = width; }
void set_height(double 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
Jul 22 '05 #3
"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(double width) { this->width = width; }
void set_height(double 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.
Jul 22 '05 #4
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(double width) { this->width = width; }
void set_height(double 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.
Jul 22 '05 #5
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).
Jul 22 '05 #6
"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 characteristically 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.

Jul 22 '05 #7
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
Jul 22 '05 #8
"osmium" <r1********@comcast.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 characteristically 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
Jul 22 '05 #9
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 characteristically 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.
Jul 22 '05 #10
"Matthias Käppler" <no****@digitalraid.com> wrote in message
news:cn*************@news.t-online.com...
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(double width) { this->width = width; }
void set_height(double height) { this->height = height; }
};

If so then why?


Poor design in terms of what? Notation? Mechanics?


Poor design in terms of lots of things maybe. In particular poor design in
terms of lacking purpose over a struct or std::pair.

Jul 22 '05 #11

"Jason Heyes" <ja********@optusnet.com.au> wrote in message
news:41**********************@news.optusnet.com.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(double width) { this->width = width; }
void set_height(double height) { this->height = height; }
};

If so then why?


Well, when you give access to all internal variables like that, it's not really
data hiding any more. However, the abstraction does allow you to change the way
you store height and width at a later time (assuming it's something that can be
converted from the doubles in the functions.) Also, using "this" isn't very
cool.
Jul 22 '05 #12

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

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.


That's completely the wrong approach. The reason you write "for the future" is
specifically because you can NOT predict the future, not because you can.
Separation of implementation and interface can have big benefits when things
change in the future.
Jul 22 '05 #13
"Jason Heyes" <ja********@optusnet.com.au> wrote in message news:<41**********************@news.optusnet.com.a u>...
class Rectangle
{
double width, height;
public:
double get_width() const { return width; }
double get_height() const { return height; }

void set_width(double width) { this->width = width; }
void set_height(double height) { this->height = height; }
};


Here's what I would have done:

#include "Helper.h"

class Rectangle {
ATTR_BBB(width, double);
ATTR_BBB(height,double);
};

I could then work with a rectangle as follows:

Rectangle r;
r.width(5.0); // sets the width to 5
cout << r.width(); // prints the width
cout << r._width; // tries to access the (protected) width member
directly.

Inside of my Helper.h file, I have definitions for automatically
declaring a protected member, along with accessor and modifier
functions. ATTR means I'm declaring an attribute, and the three Bs
are codes that determine whether the member, accessor, and modifier
functions work with base types, pointers, or references (B is for base
type, P for pointer, R for reference).

Here is the definition of ATTR_BBB:

#define ATTR_BBB(n,t) \
public: typedef t T_##n; \
t n() const { return _##n; } \
void n(t v) {_##n = v; } \
protected: t _##n

And also, FYI, here is the definition for ATTR_PRR, which has a
pointer to a type for its protected member, and the accessors and
modifiers deal with references:

#define ATTR_PRR(n,t) \
public: typedef t T_##n; \
t & n() { return * _##n; } \
const t & n() const { return * _##n; } \
void n(t & v) { _##n = & v; } \
protected: t * _##n

Cryptic, huh? Gotta love C. (## is the token pasting pre-processor
operator. _##n translates to "_width", or whatever)

Anyway, don't listen to those who tell you to just declare it as a
struct. I once made a coordinate class with public x and y members,
and, you guessed it, far into the project I came across a case that
required special handling for accessing the x and y values. If I had
been using accessors / modifiers, it would've been a trivial
modification, but instead it required a whole day of going through the
code. With inlined accessors and modifiers, there is zero performance
penalty associated with using them, and if you have a set of
accessor/modifier maker macros (like ATTR_BBB above), then you can
create them with a single line as well, making it just as terse and
readable.

Seems pretty compelling to me. Use accessor / modifier methods.

OH UNLESS UR SUPER LEET AND NEVER MAKE MISTAKES then you can do
whatever you want do it the oldsk00l C way lolol
Jul 22 '05 #14
>Well, when you give access to all internal variables like that, it's not
really data hiding any more.


No, but as you point out it does prevent clients from developing dependencies
on the implementation which might have benefits down the road.

I would not even consider this a data hiding issue. The data is part of the
abstraction and is part of the visible state. A rectangle class is not too
useful if you cannot get its dimensions. It seems more of a "implementation
hiding" issue.

When I think of data hiding I think of preventing clients from being aware of
data members that could be of no interest to them.

For example, if I were implementing a reference counted string I would provide
a method to access the strings contents but I would not have a member that
allowed the current reference count to be read. Users of the string might
choose to use a reference counted string for efficiency purposes but knowing
the reference count is probably of no value so I would keep it hidden.

Just my one cent


Jul 22 '05 #15

"DaKoadMunky" <da*********@aol.com> wrote in message
news:20***************************@mb-m29.aol.com...
Well, when you give access to all internal variables like that, it's not
really data hiding any more.


No, but as you point out it does prevent clients from developing dependencies
on the implementation which might have benefits down the road.

I would not even consider this a data hiding issue. The data is part of the
abstraction and is part of the visible state. A rectangle class is not too
useful if you cannot get its dimensions. It seems more of a "implementation
hiding" issue.


Well, sometimes "data" and "implementation" are not so black and white. Let's
say the "data" is requested in the form of a double, but stored in the form of a
long with a separate variable indicating how many decimal places exist. Is that
data, or implementation? Where do you draw the line between returning data and
returning operations based on that data? In this case it just so happens that
the information requested is the same as the information stored, but it doesn't
have to be that way for the same principles to apply. For example, let's say we
ask the rectangle its area. Is that a piece of "data" we have to store in the
object, or can it be calculated on the fly? Is there any way the client can
tell the difference? The "implementation" of whether this is stored data or
calculated on the fly can change continually, as long as there's an interface,
but to the client, it just looks like "data". If we calculated it on the fly,
would you say the data exists already, but just in a different form? Would you
be OK with not putting the area function in the interface since the client can
easily calculate it itself based on the other data available (i.e. the client
already has all the information needed about the rectangle)? Sometimes there's
a direct connection, and sometimes there isn't. But if you're saying that if an
object has data that clients need to know about it's still often a good idea to
provide an interface, I agree.
Jul 22 '05 #16

"Grey Plastic" <gr*********@hotmail.com> wrote in message
news:1d*************************@posting.google.co m...

Anyway, don't listen to those who tell you to just declare it as a
struct.


Just to be clear, a struct and a class in C++ are the same thing. Which to use?
Some use the convention that Plain Old Data should be implemented as a struct,
and otherwise it should be a class. But it makes absolutely no difference
whatsoever, technically.
Jul 22 '05 #17
gr*********@hotmail.com (Grey Plastic) wrote in message news:<1d*************************@posting.google.c om>...
[snip]
And also, FYI, here is the definition for ATTR_PRR, which has a
pointer to a type for its protected member, and the accessors and
modifiers deal with references:

#define ATTR_PRR(n,t) \
public: typedef t T_##n; \
t & n() { return * _##n; } \
const t & n() const { return * _##n; } \
void n(t & v) { _##n = & v; } \
protected: t * _##n

Cryptic, huh? Gotta love C. (## is the token pasting pre-processor
operator. _##n translates to "_width", or whatever)

[snap]

I hope you show this to every potential employer. Because they
can all use a good laugh as they stamp your resume "Rejected!"
Socks
Jul 22 '05 #18
pu*********@hotmail.com wrote:
gr*********@hotmail.com (Grey Plastic) wrote in message
news:<1d*************************@posting.google.c om>...
[snip]
And also, FYI, here is the definition for ATTR_PRR, which has a
pointer to a type for its protected member, and the accessors and
modifiers deal with references:

#define ATTR_PRR(n,t) \
public: typedef t T_##n; \
t & n() { return * _##n; } \
const t & n() const { return * _##n; } \
void n(t & v) { _##n = & v; } \
protected: t * _##n

Cryptic, huh? Gotta love C. (## is the token pasting pre-processor
operator. _##n translates to "_width", or whatever)

[snap]

I hope you show this to every potential employer. Because they
can all use a good laugh as they stamp your resume "Rejected!"
Socks


Agreed. This sort of preprocessor wizardry is not appropriate in C++.
Jul 22 '05 #19

This thread has been closed and replies have been disabled. Please start a new discussion.

Similar topics

50
by: Dan Perl | last post by:
There is something with initializing mutable class attributes that I am struggling with. I'll use an example to explain: class Father: attr1=None # this is OK attr2= # this is wrong...
13
by: Bryan Parkoff | last post by:
I have created three classes according to my own design. First class is called CMain. It is the Top Class. Second class and third class are called CMemory and CMPU. They are the sub-classes....
15
by: Steven T. Hatton | last post by:
The following may strike many of you as just plain silly, but it represents the kind of delelima I find myself in when trying to make a design decision. This really is a toy project written for...
3
by: fernandez.dan | last post by:
I'm still learning how to use Object Oriented concepts. I'm have a basic question dealing with design. I have two classes that deal with I/O pertaining to network and usb that inherit from an...
1
by: Alfonso Morra | last post by:
if I have a class template declared as ff: (BTW is this a partial specialization? - I think it is) template <typename T1, myenum_1 e1=OK, my_enum_2=NONE> class A { public: A(); virtual...
11
by: dimension | last post by:
If my intentions are to create objects that encapsulates data rows in a table, is it better to use a Struct or Class? Based on what i read, if my objects will simply have get and set methods,...
6
by: rodchar | last post by:
Hey all, I'm trying to understand Master/Detail concepts in VB.NET. If I do a data adapter fill for both customer and orders from Northwind where should that dataset live? What client is...
43
by: Tony | last post by:
I'm working with GUI messaging and note that MFC encapsulates the message loop inside of a C++ class member function. Is this somehow inherently less robust than calling the message loop functions...
6
by: JoeC | last post by:
I have a question about designing objects and programming. What is the best way to design objects? Create objects debug them and later if you need some new features just use inhereitance. Often...
29
by: Brad Pears | last post by:
Here is a simple OO design question... I have a Contract class. The user can either save an existing contract or they start off fresh with a blank contract, fill in the data and then save a...
0
marktang
by: marktang | last post by:
ONU (Optical Network Unit) is one of the key components for providing high-speed Internet services. Its primary function is to act as an endpoint device located at the user's premises. However,...
1
by: Hystou | last post by:
Overview: Windows 11 and 10 have less user interface control over operating system update behaviour than previous versions of Windows. In Windows 11 and 10, there is no way to turn off the Windows...
0
tracyyun
by: tracyyun | last post by:
Dear forum friends, With the development of smart home technology, a variety of wireless communication protocols have appeared on the market, such as Zigbee, Z-Wave, Wi-Fi, Bluetooth, etc. Each...
0
agi2029
by: agi2029 | last post by:
Let's talk about the concept of autonomous AI software engineers and no-code agents. These AIs are designed to manage the entire lifecycle of a software development project—planning, coding, testing,...
0
by: conductexam | last post by:
I have .net C# application in which I am extracting data from word file and save it in database particularly. To store word all data as it is I am converting the whole word file firstly in HTML and...
0
by: adsilva | last post by:
A Windows Forms form does not have the event Unload, like VB6. What one acts like?
0
by: 6302768590 | last post by:
Hai team i want code for transfer the data from one system to another through IP address by using C# our system has to for every 5mins then we have to update the data what the data is updated ...
1
muto222
php
by: muto222 | last post by:
How can i add a mobile payment intergratation into php mysql website.
0
bsmnconsultancy
by: bsmnconsultancy | last post by:
In today's digital era, a well-designed website is crucial for businesses looking to succeed. Whether you're a small business owner or a large corporation in Toronto, having a strong online presence...

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.