accessor/mutator - design 'flaw'

[ma740988]

I'm hoping my post here doesn't fall into the 'hard to say' category,
nonetheless I've been advised that multiple uses of accessor/mutator
(get/set) member functions can be viewed as a 'design flaw'. In that
regard I'm trying to create an 'example' class that's an alternative
to the accessor/mutator approach. To further describe the problem
consider the class BAR (below) which has a member data in_use that FOO
needs visibility into. Similarily FOO has member data 'idx' that BAR
needs visibility into.
In terms of visibility I could envision three approaches.
1. Make the data members public
2. declare BAR a friend of FOO and vise versa
3. Use Get/Set. This presumably points to a design flaw.

If memory serves me well, item 1 also suggests a 'design flaw' so I'm
left with 2 but I've often been leery of 'friends'

Any pointers/help appreaciated.

Thanks

# include<iostream>
# include "foo.h"

class BAR
{
public:
BAR() : in_use(0)
{
//std::cout << " bar's constructor " << std::endl;
}
~BAR() {}
void GetPosFbkFoo()
{
in_use ^= 1;
if (in_use)
{
int jdx = foo.GetFbk();
std::cout << jdx << std::endl;
}
else // do something else
{
foo.ComputeTorquerCmd();
}
}
private:
int in_use;
// lots more member data
FOO foo;
};

int main()
{
BAR *ptrBar = new BAR;
for (int idx(0); idx < 10; ++idx)
ptrBar->GetPosFbkFoo();
delete ptrBar;
return 0;
}

# include "foo.h"
# include <iostream>

FOO::FOO() : idx(0)
{
//std::cout << " foo's constructor called " << std::endl;
}

FOO::~FOO()
{
std::cout << " foo destructing " << std::endl;
}

int FOO::GetFbk()
{
return ++idx; // for demo purposes
}

void FOO::ComputeTorquerCmd()
{
// need visibility into the in_use flag here.
// Approach:
// 1. Make the in_use flag public;
// 2. declare BAR a friend of FOO
// 3. Use Get/Set
}


#ifndef FOO_H
#define FOO_H

class FOO
{
public:
FOO();
~FOO();
int GetFbk();
void ComputeTorquerCmd();
private:
int idx;
// more .. BAR

};

#endif

[ Spam Prevention: Replace ma740988 with mpowell to email me .. ]

 

[Dave Townsend]

I'm hoping my post here doesn't fall into the 'hard to say' category,
nonetheless I've been advised that multiple uses of accessor/mutator
(get/set) member functions can be viewed as a 'design flaw'. In that
regard I'm trying to create an 'example' class that's an alternative
to the accessor/mutator approach. To further describe the problem
consider the class BAR (below) which has a member data in_use that FOO
needs visibility into. Similarily FOO has member data 'idx' that BAR
needs visibility into.
In terms of visibility I could envision three approaches.
1. Make the data members public
2. declare BAR a friend of FOO and vise versa
3. Use Get/Set. This presumably points to a design flaw.

If memory serves me well, item 1 also suggests a 'design flaw' so I'm
left with 2 but I've often been leery of 'friends'

Any pointers/help appreaciated.

Thanks

# include<iostream>
# include "foo.h"

class BAR
{
public:
BAR() : in_use(0)
{
//std::cout << " bar's constructor " << std::endl;
}
~BAR() {}
void GetPosFbkFoo()
{
in_use ^= 1;
if (in_use)
{
int jdx = foo.GetFbk();
std::cout << jdx << std::endl;
}
else // do something else
{
foo.ComputeTorquerCmd();
}
}
private:
int in_use;
// lots more member data
FOO foo;
};

int main()
{
BAR *ptrBar = new BAR;
for (int idx(0); idx < 10; ++idx)
ptrBar->GetPosFbkFoo();
delete ptrBar;
return 0;
}

# include "foo.h"
# include <iostream>

FOO::FOO() : idx(0)
{
//std::cout << " foo's constructor called " << std::endl;
}

FOO::~FOO()
{
std::cout << " foo destructing " << std::endl;
}

int FOO::GetFbk()
{
return ++idx; // for demo purposes
}

void FOO::ComputeTorquerCmd()
{
// need visibility into the in_use flag here.
// Approach:
// 1. Make the in_use flag public;
// 2. declare BAR a friend of FOO
// 3. Use Get/Set
}


#ifndef FOO_H
#define FOO_H

class FOO
{
public:
FOO();
~FOO();
int GetFbk();
void ComputeTorquerCmd();
private:
int idx;
// more .. BAR

};

#endif

[ Spam Prevention: Replace ma740988 with mpowell to email me .. ]

Use of set/get members is not necessarily a design flaw or the sign of poor
coding,
if the setting of a property of the object causes some other side effect and
this is encapulated in the set method, then that is generally good. In the
GUI programming
world, widgets generally have a numer of getter/setters, yet setting a color
for instance will
not only set the internal color variable, it will normally cause the widget
to be repainted in
the correct color.

What is bad, is using a class as a data depository with the multitude of
settters/getters,
yet the client needs to perform the logic which should rightfully be done by
the class.

With respect to your example, it doesn't seem to have any connection with a
real-world
system so I find it impossible to help with a suggestion to improve it.


If you can state in simple words what you are trying to do, we might be
able to help model it better in C++.

dave

 

[ma740988]

Use of set/get members is not necessarily a design flaw or the sign of poor
coding, if the setting of a property of the object causes some other side
effect and this is encapulated in the set method, then that is generally
good. In the GUI programming world, widgets generally have a numer of
getter/setters, yet setting a color for instance will not only set the
internal color variable, it will normally cause the widget to be repainted in
the correct color.

Got it!! So I tried to get clarification from my adviser on what he
meant by 'design flaw' with regard to - seemingly excessive -
accessor/mutator usage.

As I - now understand things. If I have a class FOO and a class BAR.
I want 'something' of FOO in BAR, likewise something of BAR in FOO.
The preferred solution is to have a common 'stuff' class, then have
FOO and BAR inherit from the common stuff class.

What is bad, is using a class as a data depository with the multitude of
settters/getters, yet the client needs to perform the logic which should
rightfully be done by the class.

I think I see where you're going here.

 

[David Rubin]

(e-mail address removed) (ma740988) wrote in message
[snip]

As I - now understand things. If I have a class FOO and a class BAR.
I want 'something' of FOO in BAR, likewise something of BAR in FOO.
The preferred solution is to have a common 'stuff' class, then have
FOO and BAR inherit from the common stuff class.

Yes. It's called escalation. See Lakos. /david

 

[Dave Townsend]

I'm guessing, but instead of inheriting common stuff by
FOO and BAR, you would probably find aggregating or
referencing a class within FOO and BAR would be better.
Here you will have a member variable of type "common"
which you need to share between FOO and BAR. ( it might
even be the same object - you'll use a reference or pointer to
a Common object which is created somewhere else...).
Unless you need FOO and BAR to inherit from the same base
class, aggregation is usually a better approach.

ie,

class FOO
{
// constructors/destr....
private:
Common _common;

};
class BAR
{
//////
private:
Common _common;

};

#include <std_disclaimers.h>

dave.

(e-mail address removed) (ma740988) wrote in message

[snip]
As I - now understand things. If I have a class FOO and a class BAR.
I want 'something' of FOO in BAR, likewise something of BAR in FOO.
The preferred solution is to have a common 'stuff' class, then have
FOO and BAR inherit from the common stuff class.

Yes. It's called escalation. See Lakos. /david

 

[Jerry Coffin]

I'm hoping my post here doesn't fall into the 'hard to say' category,
nonetheless I've been advised that multiple uses of accessor/mutator
(get/set) member functions can be viewed as a 'design flaw'. In that
regard I'm trying to create an 'example' class that's an alternative
to the accessor/mutator approach. To further describe the problem
consider the class BAR (below) which has a member data in_use that FOO
needs visibility into. Similarily FOO has member data 'idx' that BAR
needs visibility into.

Here's where you're going wrong: neither "needs visibility into" a
variable. Rather, each needs to accomplish something.

Right now, you've described a way you've thought of to solve a
problem, but you have NOT told us about the problem itself, or even
enough about your proposed solution for us to do more than take
_really_ wild guesses at what the real problem might be.

Nobody can provide a lot of insight into solving the problem well
without knowing what the problem is.

 

[ma740988]

Here's where you're going wrong: neither "needs visibility into" a
variable. Rather, each needs to accomplish something.

Right now, you've described a way you've thought of to solve a
problem, but you have NOT told us about the problem itself, or even
enough about your proposed solution for us to do more than take
_really_ wild guesses at what the real problem might be.

Nobody can provide a lot of insight into solving the problem well
without knowing what the problem is.

Jerry, I'm unsure how much this might help but the task at hand
involves an adaptive systems. In effect lots of feedback control with
regards to closed loop servo computations. Computationally intensive
to say the least but run on DY4 Quad boards, with PowerPc's 1Gig hertz
and up. That aside, my issue is the hordes of member data (doubles)
among the usual ints (statics etc) that need to be shared (I'll
refrain from using visibility ) across multiple classes.
My initial thought was - for arguments sake - to have class
Position_Loop have a pointer to class Control_Loop and vice versa

So now

class Position_Loop
{
private:
double pos_a, pos_b, pos_c, pos_d; // lots more
// 'the a, b, c's are fictious
Control_Loop* ptr_ctrl_loop; // <- the important stuff
};

class Control_Loop
{

private:
double ctrl_a, ctrl_b, ctrl_d; // lots more
Position_Loop* ptr_pos_loop; // <- the important stuff
};


My advisor deemed said approach inane. In his view class LOS (line of
sight) needs visibility into - say member data pos_a before we drive a
servo. With that in mind I'd be required to do:

class LOS
{
private:
Position_Loop* ptr_pos_loop; // <- the important stuff
}

Whew!! When does it end. So it's back to the drawing board. My
advisor recommended a 'common_class' that'll have all the member data.
My initial reaction was that seems .. well ... 'silly'. In part
because common class serves NO useful purpose than to 'hold' member
data with a member function for 'cout's'. So now:

class Common_Class
{
public:

protected:
double ctrl_a, ctrl_b, ctrl_d; // lots more
double pos_a, pos_b, pos_c, pos_d;
}

My advisor further suggested I could inherit from common_class. Ok!!.

But I got to thinking.. How about a 'namespace'.. Except, here I'm
not sure.
In the end, I'd like to create an 'example' that I could use as a
template, mainly for future use.

 

[Jerry Coffin]

(e-mail address removed) (ma740988) wrote in message
[ ... ]

Jerry, I'm unsure how much this might help but the task at hand
involves an adaptive systems. In effect lots of feedback control with
regards to closed loop servo computations. Computationally intensive
to say the least but run on DY4 Quad boards, with PowerPc's 1Gig hertz
and up. That aside, my issue is the hordes of member data (doubles)
among the usual ints (statics etc) that need to be shared (I'll
refrain from using visibility ) across multiple classes.

So far, "adaptive" "closed loop servo computations" is the sum total
of what you've told us about the _problem_. From there, you jumped
directly back into how you've attempted to solve the problem.

My advice would be to write up something like a one-page description
of the _problem_. This is NOT allowed to contain _any_ references to
code or coding artifacts like classes or variables, nor to advice
you've gotten about those things. It needs to concentrate on the
_problem_ you're trying to solve. This should be written as
grammatical sentences in a normal _human_ language (e.g. English,
French, Russian, etc.) NOT in a programming language. Attempt to write
it on the level of explaining what you're doing to an intelligent
adult whose knowledge of computers is restricted to no more than
operating, NOT programming.

Once you've done that, you can start to attempt to identify reaonable
classes for your solution. One approach that often works reasonably
well is to look at that and underline all the nouns. Find the 5 (or
so) nouns that seem to be the most important, and there's a pretty
fair chance that those are the identities of some of your primary
classes.

Then look at the verbs, and how they relate to the nouns above. In
most cases, the verbs identify the major member functions of the
classes. The noun to which each most closely relates is an indication
of which class it will belong to.

Ultimately, I don't know for certain that you really _don't_
understand the problem you're trying to solve -- but I do know that
nothing you've posted so far gives a strong indication that you really
_do_ understand it particularly well either. I'm even more certain
that what you've posted so far doesn't describe the problem well
enough for any of _us_ to really understand it, which means we can't
do much to help solve it either.

Clear solutions derive directly from clear understanding. Without
clear understanding, a clear solution can happen only by accident.

 

[ma740988]

(e-mail address removed) (ma740988) wrote in message
[ ... ]

Jerry, I'm unsure how much this might help but the task at hand
involves an adaptive systems. In effect lots of feedback control with
regards to closed loop servo computations. Computationally intensive
to say the least but run on DY4 Quad boards, with PowerPc's 1Gig hertz
and up. That aside, my issue is the hordes of member data (doubles)
among the usual ints (statics etc) that need to be shared (I'll
refrain from using visibility ) across multiple classes.

So far, "adaptive" "closed loop servo computations" is the sum total
of what you've told us about the _problem_. From there, you jumped
directly back into how you've attempted to solve the problem.

My advice would be to write up something like a one-page description
of the _problem_. This is NOT allowed to contain _any_ references to
code or coding artifacts like classes or variables, nor to advice
you've gotten about those things. It needs to concentrate on the
_problem_ you're trying to solve. This should be written as
grammatical sentences in a normal _human_ language (e.g. English,
French, Russian, etc.) NOT in a programming language. Attempt to write
it on the level of explaining what you're doing to an intelligent
adult whose knowledge of computers is restricted to no more than
operating, NOT programming.

Once you've done that, you can start to attempt to identify reaonable
classes for your solution. One approach that often works reasonably
well is to look at that and underline all the nouns. Find the 5 (or
so) nouns that seem to be the most important, and there's a pretty
fair chance that those are the identities of some of your primary
classes.

Then look at the verbs, and how they relate to the nouns above. In
most cases, the verbs identify the major member functions of the
classes. The noun to which each most closely relates is an indication
of which class it will belong to.

Ultimately, I don't know for certain that you really _don't_
understand the problem you're trying to solve -- but I do know that
nothing you've posted so far gives a strong indication that you really
_do_ understand it particularly well either. I'm even more certain
that what you've posted so far doesn't describe the problem well
enough for any of _us_ to really understand it, which means we can't
do much to help solve it either.

I find your third and fourth paragraph intriguing, nontheless, my
issue lies with the approach from a 'software perspective'. When I
approached the problem from a C++ perspective I was asked, Why all the
abstraction? In essence, throw a C solution at 'it'. You see,
there's a tendency among us to - just make it work. For the issue of
'shared data amongst class', I refused to 'just make it work'.
Meaning, I could throw all the 'member data' in a header file, then
include the header file across the needed source files, but I didn't
think that was a ++ approach. Besides, after reading Josuttis I'm
too excited about std::string to revert back to that char * 'stuff'.

I think I've beaten this horse to death.

Thanks for your time.

 

[puppet_sock]

My advice would be to write up something like a one-page description
of the _problem_. This is NOT allowed to contain _any_ references to
code or coding artifacts like classes or variables, nor to advice
you've gotten about those things. It needs to concentrate on the
_problem_ you're trying to solve. This should be written as
grammatical sentences in a normal _human_ language (e.g. English,
French, Russian, etc.) NOT in a programming language. Attempt to write
it on the level of explaining what you're doing to an intelligent
adult whose knowledge of computers is restricted to no more than
operating, NOT programming.

I want to cheer for that advice. One of the killers in projects
I've been on is that the client does not know, or does not express
to me, or I fail to find out, exactly what is wanted. So, for
example, a project begun in October of last year has only two
weeks ago made a significant change to the written spec, with the
result I had to go back and redo quite a bit of coding. And all
my protestations that "I just did what the spec said" did not
please anybody.

Don't skimp in spending a lot of time on the spec. It will be
time well invested. If you botch this step, the rest of the
project is not going to come out too well. And do resist the
notion of specifying things in terms of how you will satisfy
the requirements. So don't write things like "we need a class to
look after the whoozits." Instead, write things like "we have
these whoozits and they have the following properties..., and
the following behaviours..., and the following requirements..."

The notion here is, solve the problem in the problem space, not
in computer code space. Once you have the solution, then you
can implement it in computer code space. You will also, fairly
inutitively but probably with a lot of work, explore the
probable changes in the near future. So, for example, you list
the properties of your whoozits. And you notice that there
are actually five possible types of whoozits, you currently
use only two, but you expect the other three to get added in
the near future. So it would be very silly not to plan for them,
and indeed the brand new "whoozit-MK-IV with gofaster stripes"
that will probably be released next month.

This is a strange experience in object oriented coding, at least
for most first time developers. The time taken in designing stuff
before anybody actually sits down and writes code may be a large
fraction of the total project time. This can be pretty scary.
If you spend one third of the project time and still have not
got a single line of code, your manager or adviser may start to
get pretty impatient. Stay the course! There's not much point in
writing code to the wrong spec.
Socks

 

[Dave Townsend]

I was curious about this snippet of code. Why do you need
access to Control_loop inside Position_loop and vice versa ?
You haven't explained the "important stuff".
I suspect from your description of feedback/control,
that when calls to Position_loop are made, control_loop
is also notified that something changed, and so on. If this is
the case, you might want to consider instead an "observer" style
pattern, so Position_loop generates notifications in a general way,
similarly for Control_loop. .
The "owners" of Control and Positon then
set up "subscriptions" between these two objects to received
notifications.
Inside control/position loops you can do various filtering/ignoring of
events as you see fit.

Decoupling in this way would make your life a lot simpler since you
can design and test each class separately, the way you are going now
you need to consider both clases at once, this is always a problem.

dave



class Position_Loop
{
private:
double pos_a, pos_b, pos_c, pos_d; // lots more
// 'the a, b, c's are fictious
Control_Loop* ptr_ctrl_loop; // <- the important stuff
};

class Control_Loop
{

private:
double ctrl_a, ctrl_b, ctrl_d; // lots more
Position_Loop* ptr_pos_loop; // <- the important stuff
};

 

[ma740988]

I was curious about this snippet of code. Why do you need
access to Control_loop inside Position_loop and vice versa ?
You haven't explained the "important stuff".
I suspect from your description of feedback/control,
that when calls to Position_loop are made, control_loop
is also notified that something changed, and so on.

For the most part.. Yes!

If this is the case, you might want to consider instead an "observer" style
pattern, so Position_loop generates notifications in a general way,
similarly for Control_loop. .
The "owners" of Control and Positon then
set up "subscriptions" between these two objects to received
notifications.

What do you mean by 'owners' and 'subscriptions'? Here's the basics
related to what I understand - learned yesterday via google search -
about the observer pattern. The observer patter is used for managing
changing relationships between objects at the message level.
Pictorially, I suspect this looks like.


|Process A| |Process B|
A A
| |
+------+-----+
|
|Observer|

So now, two relationships A & B and an observer. At the source level a
concrete example on this ... well, now I'm in over my head. Assuming
I'm not sadly mistaken the 'owners' would be the objects for process A
and process B and the 'subscription' would be the observer?

Inside control/position loops you can do various filtering/ignoring of
events as you see fit.

Decoupling in this way would make your life a lot simpler since you
can design and test each class separately, the way you are going now
you need to consider both clases at once, this is always a problem.

Makes sense.