How to avoid repeating code?

[Phlip]

BTW, this is a *great* example. It shows exactly why other peoples
suggestion of making the contained objects public is a bad idea.

Sometimes, the design you suggested is bad too. That's why we are engineers;
to make tradeoffs, and find the sweet spots between the bad things.

 

[Noah Roberts]

That argument is "appeal to authority". No peer-reviewed author you know of
has proposed such a rule.

That isn't an appeal to authority.

http://www.nizkor.org/features/fallacies/appeal-to-authority.html

Now, if you can show that there is a rule behind the rule then so be
it. The "rule" you are talking about is the concept of information
hiding:

http://en.wikipedia.org/wiki/Information_hiding

Now, since you have brought up logical fallacy, albeit wrongly, then
I'll assume you are aware that it is on you to show that such a "rule
behind the rule" indeed exists as it is your claim.

I appreciate your attention to the entry-level guidelines of programming,
yet knowing their meaning is more important than such knee-jerk reactions to
violating them.

You seem to be assuming you are the one that has the correct meaning in
mind. I certainly have my doubts on that and that is in fact what this
whole conversation is about...so that makes this new argument of yours
circular.

Oh, BTW, the programming author Ward Cunningham sez to relax the private
data rules.

That _is_ an appeal to authority. Thank you for the illustration.

That is indeed how the expression "I look forward to pairing with you on
that" is used. (According to several authors...)

Well, maybe you understood my meaning, and maybe not.

They can fend for themselves. They cannot, for example, be driven into an
incorrect state, if _their_ members are private.

If you say so, can you show it? What if I show you how they can be:

parent.child().~ChildClassName();

They might indeed be driven into a state that CA does not expect.

Which means the concequences for exposing them are no less severe.

Yet they

might also if they were private. And they might have a reference to the CA,
so they can inform it of their state.

So we are back to the Hollywood Principle again...

Such application is a stretch and I don't recall ever being there to
begin with.

 

[Daniel T.]

My goodness, Danny, then they shouldn't befriend CA! Among other reasons,
they have closed source!

However, if they were vector and list, then CA should not expose them, not
even by an accessor to a constant reference. 3rd party classes have
super-wide interfaces, and we want to narrow the interfaces around the
region of CA, right?

Right. Which was my point.

 

[Daniel T.]

Sometimes...

Yes, thanks for that. I should have said, "... is sometimes a bad idea."

 

[Stuart Golodetz]

I appreciate your attention to the entry-level guidelines of programming,
yet knowing their meaning is more important than such knee-jerk reactions
to violating them.

Of course you're right that it's important to know why particular guidelines
exist. Similarly, you're right that there are times when it's best to break
them, for instance when not doing so would make the code substantially more
complicated for no reason. I'm not convinced, on the other hand, that
capriciously breaking them, on the basis of being beyond all that "newbie"
stuff, is such a good idea. In my experience (albeit limited) such an
approach usually comes back and bites you. I suspect (indeed I'd be
interested to find out one way or the other) that many of the best
programmers strongly adhere to guidelines and convention unless there is a
compelling reason to do otherwise. It's surprising how often you find that
an "avant garde" approach to a problem is just one of many wrong approaches.
YMMV.

Regards,
Stu

<snip>

 

[Phlip]

It's surprising how often you find that an "avant garde" approach to a
problem is just one of many wrong approaches. YMMV.

One thing TDD users often discover is much of the static type checking they
formerly relied on now just gets in the way.

And I didn't advise to capriciously make private variables public; I advised
that if private members are themselves encapsulated objects, then making
them public is lower risk.

 

[Stuart Golodetz]

One thing TDD users often discover is much of the static type checking
they formerly relied on now just gets in the way.

Fair enough I find both testing and static type checking to be quite
useful, but everyone's got their own way of doing things.

And I didn't advise to capriciously make private variables public; I
advised that if private members are themselves encapsulated objects, then
making them public is lower risk.

Yes, I realised. I think I was being a little bit pompous before, sorry!
That aside, though, I'm not sure I agree with you on this one. The purpose
of making things private is as much to keep the containing object in a
consistent state as it is to keep the contained objects in such a state.
Making them public may not allow the client to mess up the contained object,
but they may be able to modify it in such a way that the container is no
longer in a consistent state. This isn't a good thing, on the whole. Of
course, if the container is an aggregate, it's less of an issue.

Cheers,
Stu

 

[Jerry Coffin]

[ ... ]

And I didn't advise to capriciously make private variables public; I advised
that if private members are themselves encapsulated objects, then making
them public is lower risk.

What it really comes down to is this: public vs. private isn't really
the issue. The real issue is one of maintaining control over the values
that are assigned to the variable in question. When you're dealing with
something like a raw int, the only way to keep any control over it is to
make it private and put that control into the accessor functions. When
you're dealing with something that already provides suitable control,
you rarely gain much by using accessors to impose the control
externally.

 

[loop]

Yes, thanks for that. I should have said, "... is sometimes a bad idea."

true, my professor Dr. Robert once said so too :-D
Dawei Mark Gay

 

[Phlip]

That aside, though, I'm not sure I agree with you on this one. The purpose
of making things private is as much to keep the containing object in a
consistent state as it is to keep the contained objects in such a state.

That's the primary syntactic reason.

The primary semantic reason is to help us design client objects that know
nothing about these variables, even their existences.

Making them public may not allow the client to mess up the contained
object, but they may be able to modify it in such a way that the container
is no longer in a consistent state. This isn't a good thing, on the whole.

Even if they are private, their state might derail. Static typechecking is
only a secondary defensive measure.

Test cases and higher-level modularization are primary defensive measures.

Of course, if the container is an aggregate, it's less of an issue.

Huh? You mean that, given std::list<foo>, the list can't grab the foo's
privates? That can't be what you mean! (And I know list ain't an
aggregate...)

 

[Stuart Golodetz]

That's the primary syntactic reason.

The primary semantic reason is to help us design client objects that know
nothing about these variables, even their existences.

Yes, that's certainly true. (I wasn't intending to enumerate all the (many)
good reasons for making things private. My comment was a relative one.)

Even if they are private, their state might derail. Static typechecking is
only a secondary defensive measure.

That's also true. Why "typechecking" rather than access-checking out of
interest? The two things seem to be distinct.

Test cases and higher-level modularization are primary defensive measures.


Huh? You mean that, given std::list<foo>, the list can't grab the foo's
privates? That can't be what you mean! (And I know list ain't an
aggregate...)

No, that wasn't what I meant. What I meant was things like:

struct Point
{
int x, y;
};

for instance, as opposed to:

class Point
{
private:
int x, y;
public:
int get_x() const { return x; }
// etc. (includes all manner of things which would make the class
actually usable)
};

In cases like this, where it's ok to modify the internal state of a Point,
it's not such a problem. There is a case to be made for Point objects being
immutable here, actually, but that's another matter. By immutable, I mean
lacking in mutators and having instead methods like, say:

Point translate(int dx, int dy) { return Point(x+dx,y+dy); }

etc.

I'm not sure I subscribe to that point of view entirely, but that's another
matter.

Regarding your somewhat humorous list example, I'm not actually sure what
you're getting at, could you explain? I'm both amused and bemused at once

Cheers,
Stu

 

[Stuart Golodetz]

[ ... ]

And I didn't advise to capriciously make private variables public; I
advised
that if private members are themselves encapsulated objects, then making
them public is lower risk.

What it really comes down to is this: public vs. private isn't really
the issue. The real issue is one of maintaining control over the values
that are assigned to the variable in question. When you're dealing with
something like a raw int, the only way to keep any control over it is to
make it private and put that control into the accessor functions. When
you're dealing with something that already provides suitable control,
you rarely gain much by using accessors to impose the control
externally.

It's worth observing, though, that which values that can be assigned to the
variable in question often depends on the containing object. If that's not
the case in a particular instance, then so be it, but if it is then you
would be unwise to make the variable public. The key phrase in what you were
saying is "something that already provides suitable control" - a class
doesn't know what other class might contain an instance of it, so it
literally can't provide such suitable control. Only the containing class
knows the restrictions which need to be applied to the contained instance.

Regards,
Stu

 

[Stuart Golodetz]

[ ... ]

And I didn't advise to capriciously make private variables public; I
advised
that if private members are themselves encapsulated objects, then making
them public is lower risk.

What it really comes down to is this: public vs. private isn't really
the issue. The real issue is one of maintaining control over the values
that are assigned to the variable in question. When you're dealing with
something like a raw int, the only way to keep any control over it is to
make it private and put that control into the accessor functions. When
you're dealing with something that already provides suitable control,
you rarely gain much by using accessors to impose the control
externally.

--
Later,
Jerry.

The universe is a figment of its own imagination.

While I'm thinking about it, I think the key word which I've been missing
here is "invariants". Suppose we have the following situation:

class Y
{
//...
};

class X
{
//...
public:
Y y;
//...
};

(The comments in the above indicate (for our purposes, arbitrary) other
code, incidentally). Now, suppose both X and Y have datatype invariants
which all their methods studiously maintain. What I'm suggesting is that
uses of the y member of an instance of X may result in X's datatype
invariant being violated. It's theoretically possible that Y could be
written in such a way that it would ensure that X's invariant was
maintained, but that would make life very difficult for the author of Y, who
might not even know that X exists. Of course, it's not necessarily the case
that uses of y will cause X's invariant to be violated; that depends on what
X's invariant actually is and what the uses of y do. If, as in the Point
example I gave in my other post, there is no invariant, then maintaining
internal consistency is not a motive to make the member variables private.
(There may be other motives.)

A little disclaimer: Since both you and Phlip seem to agree on this, I have
to wonder whether I'm talking rubbish here?! Sorry if I am; perhaps I'm
missing the point. If so, could you give a small, concrete example of what
you mean?

Regards,
Stu

 

[Alf P. Steinbach]

* shuisheng:

class CA1
{
public:
void func1( );
void func2( );
...
};

class CA2
{
public:
void func1( );
void func2( );
...
};

class CA
{
public:
void a1_func1( a1.func1( ) );
void a1_func2( a1.func2( ) );
...
void a2_func1( a2.func1( ) );
void a2_func2( a2.func2( ) );
...
private:
CA1 a1;
CA2 a2;
...
}

I appreciate your help.

You have not explained the purpose of class CA, which as it stands can
be entirely eliminated.

The discussion so far has assumed various scenarios, like (1) class CA
presents only a subset of CA1+CA2, or (2) class CA enforces some
relationship between members a1 and a2 (a class invariant). But I think
the most likely is (3) class CA really serves no useful purpose.

In that case, eliminate it.

 

[Jerry Coffin]

[ ... ]

It's worth observing, though, that which values that can be assigned to the
variable in question often depends on the containing object.

At least in my experience, it often depends on the containing _class_,
and less often on the actual containing object (i.e. the instance as
opposed to the class). That may easily be what you meant, but (at least
to me) it's a significant distinction.

If that's not
the case in a particular instance, then so be it, but if it is then you
would be unwise to make the variable public. The key phrase in what you were
saying is "something that already provides suitable control" - a class
doesn't know what other class might contain an instance of it, so it
literally can't provide such suitable control. Only the containing class
knows the restrictions which need to be applied to the contained instance.

IME, at least 80% of the time, all that's really needed is fairly simple
range-checking. In this case, it's pretty easy to isolate the work into
one small, fairly simple class. (I've posted roughly similar code to
this previously, but this is slightly different from them -- this
supports floating point types, and finally gets around to fixing a
problem I introduced years ago, of throwing an exception upon attempting
to extract an out-of-range value, rather than setting the stream's fail
bit as it should).

#include <exception>
#include <iostream>
#include <functional>
#include <ios>

template <class T, class less=std::less<T> >
class bounded {
const T lower_, upper_;
T val_;

bool check(T const &value) {
return less()(value, lower_) || less()(upper_, value);
}

void assign(T const &value) {
if (check(value))
throw std::domain_error("Out of Range");
val_ = value;
}

public:
bounded(T const &lower, T const &upper)
: lower_(lower), upper_(upper) {}

bounded(bounded const &init) { assign(init); }

bounded &operator=(T const &v) { assign(v); return *this; }

operator T() const { return val_; }

friend std::istream &operator>>(std::istream &is, bounded &b) {
T temp;
is >> temp;

if (b.check(temp))
is.setstate(std::ios::failbit);
else
b.val_ = temp;
return is;
}
};

Now, the parent class uses something like:

struct geoPosition {
bounded<double> latitude(-90.0, 90.0);
bounded<double> longitude(-180.0, 180.0);
// ...
};

The range is now directly documented in the definition, rather than
being spread throughout the code of the owning class. This code itself
is simple enough that testing and verification is trivial, and it
isolates the range checking, so changes in the owning class can't
accidentally by-pass the range-checking anywhere, or things like that.

 

[Phlip]

That's also true. Why "typechecking" rather than access-checking out of
interest? The two things seem to be distinct.

Their common aspect is they are a secondary defensive measure. Static
checking of a program source, at compile-time.

...What I meant was things like:

struct Point
{
int x, y;
};

for instance, as opposed to:

class Point
{
private:
int x, y;
public:
int get_x() const { return x; }
// etc. (includes all manner of things which would make the class
actually usable)
};

In cases like this, where it's ok to modify the internal state of a Point,
it's not such a problem.

If the struct Point is itself private to a known capsule, such as a module
or package.

Regarding your somewhat humorous list example, I'm not actually sure what
you're getting at, could you explain? I'm both amused and bemused at once

My list<foo>? I forgot some engineers use "aggregate" to mean "dumb struct".

In generics, the container knows nothing about the contained, except its
primitive interface. ==, >, =, etc.

 

[Jerry Coffin]

[ ... ]

While I'm thinking about it, I think the key word which I've been missing
here is "invariants". Suppose we have the following situation:

class Y
{
//...
};

class X
{
//...
public:
Y y;
//...
};

(The comments in the above indicate (for our purposes, arbitrary) other
code, incidentally). Now, suppose both X and Y have datatype invariants
which all their methods studiously maintain. What I'm suggesting is that
uses of the y member of an instance of X may result in X's datatype
invariant being violated.

The question in my mind at that point is whether Y is really the correct
data type to be using in X. If X needs to enforce constraints on y
beyond those inherent in Y, then it seems fairly likely to me that Y
really isn't the correct data type. The cure would be to create a class
(possibly a derivative of Y) that really is the correct data type, and
then use it.

There are situations that are a bit more difficult. The most obvious is
when/if you need to enforce a constraint that is based on a relationship
between two different objects. For example, consider a pair of X and Y
coordinates, but you're only allowed to create a position that's inside
of a particular country (or state, county, city, etc.) Since most of the
boundaries aren't going to be precise rectangles, the permissible values
of X and Y are interrelated, and you can't enforce either separately
from the other.

IME, this can still usually be handled by creating a suitable data type.
Range checking might become somewhat (or even extremely) complex --
perhaps requiring an extensive data structure representing a map to tell
whether a particular value is in bounds or not. Nonetheless, the idea
remains the same -- first, realizing that if two such pieces of data are
really so closely related, they should probably be represented as a
single value of some composite type, and second, creating a class
specifically to represent that type.

A little disclaimer: Since both you and Phlip seem to agree on this, I have
to wonder whether I'm talking rubbish here?! Sorry if I am; perhaps I'm
missing the point. If so, could you give a small, concrete example of what
you mean?

I don't think you're talking rubbish; I think it's more a difference in
emphasis. As much as possible, I prefer to keep the requirements on any
one class small, simple, easily understood and easily articulated. IMO,
enforcing a set of constraints is a sufficient responsibility for a
class. In fact, under some circumstances, it's worth breaking it up into
two classes -- one class (similar to the one I posted yesterday) that
actually enforces the constraint, and a second policy class that's
passed as a parameter to the first that actually determines what's in or
out of bounds for the particular type in question.

 

[Stuart Golodetz]

Their common aspect is they are a secondary defensive measure. Static
checking of a program source, at compile-time.


If the struct Point is itself private to a known capsule, such as a module
or package.

Certainly if it's private I wouldn't worry too much about it. With something
as simple as Point, I'd be tempted to think that it wouldn't matter even if
it was public. Point itself doesn't have a datatype invariant, and if you
wrote it instead as a template, e.g.

template <typename T>
struct Point
{
T x, y;
};

then I can't think of any reason why the variables in it would ever change.
There's no real need to encapsulate with something like this. Well, perhaps
it could be argued both ways, I don't know.

My list<foo>? I forgot some engineers use "aggregate" to mean "dumb
struct".

Engineer?! I'm a computer scientist! Oh, the shame of it... (Just
kidding, incidentally.)

In generics, the container knows nothing about the contained, except its
primitive interface. ==, >, =, etc.

True enough. Speaking of which, I still think it's a shame that you can't
really express constraints on contained types in C++. You can document
requirements, certainly, but if they're accidentally violated you usually
end up with a fairly obscure template error. Ah well.

Regards,
Stu

 

[Stuart Golodetz]

[ ... ]

It's worth observing, though, that which values that can be assigned to
the
variable in question often depends on the containing object.

At least in my experience, it often depends on the containing _class_,
and less often on the actual containing object (i.e. the instance as
opposed to the class). That may easily be what you meant, but (at least
to me) it's a significant distinction.

I confess I didn't think too hard about it when I wrote it, but thinking
about it now I believe I actually did mean the containing object. Consider
this example (it may not be terribly good code, it's just for the purposes
of illustration):

class Rect
{
// DATATYPE INVARIANT: left <= right && top <= bottom

private:
int m_left, m_right, m_top, m_bottom;

void check_invariant(int left, int right, int top, int bottom)
{
if(left > right || top > bottom) throw ArbitraryException();
}

public:
void set_left(int left)
{
check_invariant(left, m_right, m_top, m_bottom);
m_left = left;
}

// etc.
};

In this case, the acceptable values for each of the private variables
depends on one of the other private variables. Thus what I can set m_left to
depends on the value of m_right in a given Rect instance. I can't enforce
this if m_left is public.

IME, at least 80% of the time, all that's really needed is fairly simple
range-checking. In this case, it's pretty easy to isolate the work into
one small, fairly simple class. (I've posted roughly similar code to
this previously, but this is slightly different from them -- this
supports floating point types, and finally gets around to fixing a
problem I introduced years ago, of throwing an exception upon attempting
to extract an out-of-range value, rather than setting the stream's fail
bit as it should).

#include <exception>
#include <iostream>
#include <functional>
#include <ios>

template <class T, class less=std::less<T> >
class bounded {
const T lower_, upper_;
T val_;

bool check(T const &value) {
return less()(value, lower_) || less()(upper_, value);
}

void assign(T const &value) {
if (check(value))
throw std::domain_error("Out of Range");
val_ = value;
}

public:
bounded(T const &lower, T const &upper)
: lower_(lower), upper_(upper) {}

bounded(bounded const &init) { assign(init); }

bounded &operator=(T const &v) { assign(v); return *this; }

operator T() const { return val_; }

friend std::istream &operator>>(std::istream &is, bounded &b) {
T temp;
is >> temp;

if (b.check(temp))
is.setstate(std::ios::failbit);
else
b.val_ = temp;
return is;
}
};

Now, the parent class uses something like:

struct geoPosition {
bounded<double> latitude(-90.0, 90.0);
bounded<double> longitude(-180.0, 180.0);
// ...
};

The range is now directly documented in the definition, rather than
being spread throughout the code of the owning class. This code itself
is simple enough that testing and verification is trivial, and it
isolates the range checking, so changes in the owning class can't
accidentally by-pass the range-checking anywhere, or things like that.

Well I like the above code I also think I can see how you could modify it
to take account of the situation I gave above. I'm going to give it a try
now, actually...

Cheers,
Stu

 

[Stuart Golodetz]

[ ... ]

It's worth observing, though, that which values that can be assigned to
the
variable in question often depends on the containing object.

At least in my experience, it often depends on the containing _class_,
and less often on the actual containing object (i.e. the instance as
opposed to the class). That may easily be what you meant, but (at least
to me) it's a significant distinction.

I confess I didn't think too hard about it when I wrote it, but thinking
about it now I believe I actually did mean the containing object. Consider
this example (it may not be terribly good code, it's just for the purposes
of illustration):

class Rect
{
// DATATYPE INVARIANT: left <= right && top <= bottom

Oops, I clearly meant m_left <= m_right && m_top <= m_bottom. Never mind

private:
int m_left, m_right, m_top, m_bottom;

Well I like the above code I also think I can see how you could modify
it to take account of the situation I gave above. I'm going to give it a
try now, actually...

Cheers,
Stu

Ok, my attempt looks something like the following. It's not as clear as it
could be, but it does work (I think!)

#include <istream>
#include <boost/shared_ptr.hpp>

template <typename T, typename CHECKER>
class Checked
{
private:
T m_t;
boost::shared_ptr<CHECKER> m_checker;

void assign(const T& t)
{
if(check(t)) m_t = t;
else throw std::domain_error("Invalid assignment");
}

bool check(const T& t)
{
if(m_checker.get() != NULL) return m_checker->is_ok(t);
else return true;
}

public:
Checked(const T& t)
{
assign(t);
}

Checked& operator=(const T& rhs)
{
assign(rhs);
return *this;
}

operator const T&() const
{
return m_t;
}

friend std::istream& operator>>(std::istream& is, Checked& c)
{
T t;
is >> t;
if(check(t)) m_t = t;
else is.setstate(std::ios::failbit);
return is;
}

void set_checker(CHECKER *pChecker)
{
m_checker.reset(pChecker);
if(check(m_t) == false) throw std::domain_error("Check violated");
}
};

template <typename T>
class LBoundChecker
{
private:
const T& m_lbound;
public:
LBoundChecker(const T& lbound)
: m_lbound(lbound)
{}

bool is_ok(const T& t)
{
return t >= m_lbound;
}
};

template <typename T>
class UBoundChecker
{
private:
const T& m_ubound;
public:
UBoundChecker(const T& ubound)
: m_ubound(ubound)
{}

bool is_ok(const T& t)
{
return t <= m_ubound;
}
};

struct SimpleTest
{
// DATATYPE INVARIANT: m_left <= m_right

Checked<int,UBoundChecker<int> > m_left;
Checked<int,LBoundChecker<int> > m_right;

SimpleTest() // establish the invariant arbitrarily
: m_left(0),
m_right(1)
{
m_left.set_checker(new UBoundChecker<int>(m_right));
m_right.set_checker(new LBoundChecker<int>(m_left));
}
};

I'm not sure I like all the set_checker() stuff, but otherwise problems
arise because I can't initialise both m_left and m_right at once. I think
your point that it can be done is made, though I'm sure it could be
implemented better with either more thought or a different programmer...

Regards,
Stu

<snip>