By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
446,305 Members | 1,588 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 446,305 IT Pros & Developers. It's quick & easy.

design question

P: n/a
This is a question related to my previous post titled "private member
functions:"

I recently saw that a colleage of mine submitted code which defines a
class Interface as follows[1]:

class Interface {
public:
Interface(UINT maxidx);

void set(UINT idx, UCHAR data);
/* ... */
private:
UINT d_maxidx;

bool isInRange(UINT idx) { return idx < d_maxidx; }
};

My first reaction to this code is that isInRange should have been
defined as a static function in the Interface.cc translation unit rather
than a member function:

static bool isInRange(UINT idx, UINT max) { return idx < max; }

Is my reaction correct? Any opinions?

/david

[1] simplified from original version

--
Andre, a simple peasant, had only one thing on his mind as he crept
along the East wall: 'Andre, creep... Andre, creep... Andre, creep.'
-- unknown
Jul 19 '05 #1
Share this Question
Share on Google+
10 Replies


P: n/a
David Rubin wrote:
I recently saw that a colleage of mine submitted code which defines a
class Interface as follows[1]:
[snip - class definition] My first reaction to this code is that isInRange should have been
defined as a static function in the Interface.cc translation unit rather
than a member function:


Forgot to mention that this class will not be used as a base class.

/david

--
Andre, a simple peasant, had only one thing on his mind as he crept
along the East wall: 'Andre, creep... Andre, creep... Andre, creep.'
-- unknown
Jul 19 '05 #2

P: n/a
"David Rubin" <bo***********@nomail.com> wrote...
This is a question related to my previous post titled "private member
functions:"

I recently saw that a colleage of mine submitted code which defines a
class Interface as follows[1]:

class Interface {
public:
Interface(UINT maxidx);

void set(UINT idx, UCHAR data);
/* ... */
private:
UINT d_maxidx;

bool isInRange(UINT idx) { return idx < d_maxidx; }
};

My first reaction to this code is that isInRange should have been
defined as a static function in the Interface.cc translation unit rather
than a member function:

static bool isInRange(UINT idx, UINT max) { return idx < max; }

Is my reaction correct? Any opinions?


There is no "correct" or "incorrect" reaction. It's your reaction,
nobody is going to tell you how to react.

As to the interface, my reaction to your reaction is "what the hell
is wrong with a non-static member?"

Victor
Jul 19 '05 #3

P: n/a
Victor Bazarov wrote:
class Interface {
public:
Interface(UINT maxidx);

void set(UINT idx, UCHAR data);
/* ... */
private:
UINT d_maxidx;

bool isInRange(UINT idx) { return idx < d_maxidx; }
};

My first reaction to this code is that isInRange should have been
defined as a static function in the Interface.cc translation unit rather
than a member function:

static bool isInRange(UINT idx, UINT max) { return idx < max; }

[snip] As to the interface, my reaction to your reaction is "what the hell
is wrong with a non-static member?"


It seems to pollute the interface. It's good to do error checking, but
why should this be an explicit part of the class definition? (Note my
self-reply that this class will only be used as a leaf.)

/david

--
Andre, a simple peasant, had only one thing on his mind as he crept
along the East wall: 'Andre, creep... Andre, creep... Andre, creep.'
-- unknown
Jul 19 '05 #4

P: n/a
David Rubin wrote:
> class Interface {
> public:
> Interface(UINT maxidx);
>
> void set(UINT idx, UCHAR data);
> /* ... */
> private:
> UINT d_maxidx;
>
> bool isInRange(UINT idx) { return idx < d_maxidx; }
> };
>
> My first reaction to this code is that isInRange should have been
> defined as a static function in the Interface.cc translation unit rather
> than a member function:
>
> static bool isInRange(UINT idx, UINT max) { return idx < max; }


[snip]
As to the interface, my reaction to your reaction is "what the hell
is wrong with a non-static member?"


It seems to pollute the interface. It's good to do error checking, but
why should this be an explicit part of the class definition? (Note my
self-reply that this class will only be used as a leaf.)
...


Hm... I don't understand how are you planning to implement it as a
'static' function when it needs access to the 'd_maxidx' member of a
concrete object. Are you suggesting that 'd_maxidx' should also be
pulled out of 'Interface' and made a 'static' variable?

--
Best regards,
Andrey Tarasevich
Brainbench C and C++ Programming MVP

Jul 19 '05 #5

P: n/a

"David Rubin" <bo***********@nomail.com> wrote in message
news:3F***************@nomail.com...
This is a question related to my previous post titled "private member
functions:"

I recently saw that a colleage of mine submitted code which defines a
class Interface as follows[1]:

class Interface {
public:
Interface(UINT maxidx);

void set(UINT idx, UCHAR data);
/* ... */
private:
UINT d_maxidx;

bool isInRange(UINT idx) { return idx < d_maxidx; }
};

My first reaction to this code is that isInRange should have been
defined as a static function in the Interface.cc translation unit rather
than a member function:

static bool isInRange(UINT idx, UINT max) { return idx < max; }

Is my reaction correct? Any opinions?


I think the point is you'd have to know what max is, and what if you don't?
In other words, there could be multiple instances of class Interface lying
around at any point in time. How are you going to keep track of the max
value for all of them? Isn't it better to let each instance keep track of
this number, without you having to do that work? That is, after all, one of
the basic principles of OO programming. There's nothing wrong with a static
function per se, although I have to question whether or not you really need
an Interface class to tell you if one number is bigger than another number.
Jul 19 '05 #6

P: n/a

"David Rubin" <bo***********@nomail.com> wrote in message
news:3F***************@nomail.com...

It seems to pollute the interface. It's good to do error checking, but
why should this be an explicit part of the class definition?


Because the max number is an explicit part of the class definition, which is
exactly where it belongs. A class is data along with the functions that
operate on that data, encapsulated. If the data can't be static, then why
should be function be? I'm not saying there should never be static
functions, but they are usually best for when they don't need any data that
is specific to any one instance of the class.
Jul 19 '05 #7

P: n/a
Andrey Tarasevich wrote:

David Rubin wrote:
> class Interface {
> public:
> Interface(UINT maxidx);
>
> int set(UINT idx, UCHAR data); [change! ^^^] > /* ... */
> private:
> UINT d_maxidx;
>
> bool isInRange(UINT idx) { return idx < d_maxidx; }
> }; > My first reaction to this code is that isInRange should have been
> defined as a static function in the Interface.cc translation unit rather
> than a member function: > static bool isInRange(UINT idx, UINT max) { return idx < max; }

[snip] Hm... I don't understand how are you planning to implement it as a
'static' function when it needs access to the 'd_maxidx' member of a
concrete object. Are you suggesting that 'd_maxidx' should also be
pulled out of 'Interface' and made a 'static' variable?


I was imagining something like this:

/* Interface.cc */

static bool isInRange(UINT idx, UINT max) { return idx < max; }

int Interface::set(UINT idx, UCHAR data)
{
int r = ERROR;

if(isInRange(idx, d_maxidx)){
/* non-trivial code involving data */
r = OK;
}
return r;
}

isInRange does not appear in the Interface header file at all. This way,
a proliferation of validation functions will not affect the class
interface; they are merely implementation details. The fact that
Interface::set, for example, returns a (documented) error condition is
enough to suggest to implementors that some sort of validation must be
done. However, I'm conflicted about imposing the details on Interface
clients.

/david

--
Andre, a simple peasant, had only one thing on his mind as he crept
along the East wall: 'Andre, creep... Andre, creep... Andre, creep.'
-- unknown
Jul 19 '05 #8

P: n/a
jeffc wrote:
My first reaction to this code is that isInRange should have been
defined as a static function in the Interface.cc translation unit rather
than a member function:

[snip] I think the point is you'd have to know what max is, and what if you don't?
In other words, there could be multiple instances of class Interface lying
around at any point in time. How are you going to keep track of the max
value for all of them? Isn't it better to let each instance keep track of
this number, without you having to do that work? That is, after all, one of
the basic principles of OO programming. There's nothing wrong with a static
function per se, although I have to question whether or not you really need
an Interface class to tell you if one number is bigger than another number.


Most people seem to have misread my original comment, quoted above. I am
talking about a static function in a translation unit, not a class
function (declared as static). I think my response to Andrey Tarasevich
(elsethread) explains the point a little more clearly.

/david

--
Andre, a simple peasant, had only one thing on his mind as he crept
along the East wall: 'Andre, creep... Andre, creep... Andre, creep.'
-- unknown
Jul 19 '05 #9

P: n/a

"David Rubin" <bo***********@nomail.com> wrote in message
news:3F***************@nomail.com...
jeffc wrote:
My first reaction to this code is that isInRange should have been
defined as a static function in the Interface.cc translation unit rather than a member function:
[snip]
I think the point is you'd have to know what max is, and what if you
don't? In other words, there could be multiple instances of class Interface lying around at any point in time. How are you going to keep track of the max
value for all of them? Isn't it better to let each instance keep track of this number, without you having to do that work? That is, after all, one of the basic principles of OO programming. There's nothing wrong with a static function per se, although I have to question whether or not you really need an Interface class to tell you if one number is bigger than another

number.
Most people seem to have misread my original comment, quoted above. I am
talking about a static function in a translation unit, not a class
function (declared as static). I think my response to Andrey Tarasevich
(elsethread) explains the point a little more clearly.


I didn't know that, but even so, my advice still stands, except for the
second half of the last sentence.
Jul 19 '05 #10

P: n/a
David Rubin wrote:


Most people seem to have misread my original comment, quoted above. I am
talking about a static function in a translation unit, not a class
function (declared as static). I think my response to Andrey Tarasevich
(elsethread) explains the point a little more clearly.


I'd say that this was an implementation detail that ought to
be kept out of the class header.

I suspect that it is being used to check the 'idx' value
being passed to set(). Being private clients are unable to
use it to check that 'idx' is ok or not.

I suppose that UINT is some unsigned integer - do you really
want a 4Gb range, wont 2Gb suffice? In general 'unsigned' is
a nuisence because you can't check if the value is negative
or not, and the compiler will silently promote an int to
unsigned:

int main()
{
int i = -1;
unsigned ui = i;
return 0;
}

wont generate a warning on some compilers.

More importantly this isInRange method is an example of what
I was talking about in your post about private members
functions. It has utility outside of the class, particularly
if it takes an upper and lower value like:

bool isInRange(int value, int min_value, int max_value) {
return value >= min_value && value <= max_value;
}

you might want to only use greater than and less than though.

The point being is that often these 'private methods' have a
utility outside of the class they were originally written for.

Jul 19 '05 #11

This discussion thread is closed

Replies have been disabled for this discussion.