Traversing members of a class with a pointer

[Glen Dayton]

While looking at some old code I ran across a snippet that
increments a pointer to access different members of a structure:
....

struct Badness {
std::string m_begin;
std::string m_param1;
std::string m_param2;
std::string m_end;

Badness();
};
....
Badness badness;

std::string *it = &badness.m_begi n;

for (++it; it != &badness.m_e nd; ++it) {
std::cout << it->c_str() << std::endl;
}

It just looks wrong. It depends on all of the members of class
or structure to be consistent so its bad from a maintenance
standpoint. It seems evil to use a pointer beyond the specific
object for which it is defined.

Looking wrong and evil, though, are not rational arguments that
would allow me to convince my technical lead that this code
needs to be changed sooner than later.

Anyone have good talking points?

/Glen

[Kai-Uwe Bux]

Glen Dayton wrote:

While looking at some old code I ran across a snippet that
increments a pointer to access different members of a structure:
...

struct Badness {
std::string m_begin;
std::string m_param1;
std::string m_param2;
std::string m_end;

Badness();
};
...
Badness badness;

std::string *it = &badness.m_begi n;

for (++it; it != &badness.m_e nd; ++it) {
std::cout << it->c_str() << std::endl;
}

It just looks wrong. It depends on all of the members of class
or structure to be consistent so its bad from a maintenance
standpoint. It seems evil to use a pointer beyond the specific
object for which it is defined.

Looking wrong and evil, though, are not rational arguments that
would allow me to convince my technical lead that this code
needs to be changed sooner than later.

Anyone have good talking points?

What about the undefined behavior that the code has; does that count?
Best

Kai-Uwe Bux

[Pete Becker]

Glen Dayton wrote:

>
Looking wrong and evil, though, are not rational arguments that would
allow me to convince my technical lead that this code needs to be
changed sooner than later.

If it works, it works. The behavior isn't defined by the C++ standard,
but I haven't run into a compiler where it won't do just what it ooks
like it does.

--

-- Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com)
Author of "The Standard C++ Library Extensions: a Tutorial and
Reference." (www.petebecker.com/tr1book)

[Glen Dayton]

Pete Becker wrote:

Glen Dayton wrote:

>>
Looking wrong and evil, though, are not rational arguments that would
allow me to convince my technical lead that this code needs to be
changed sooner than later.

If it works, it works. The behavior isn't defined by the C++ standard,
but I haven't run into a compiler where it won't do just what it ooks
like it does.

That it is undefined by the standard is one problem. It means
at a future date the code will stop working. That it works is
another part of the problem, because it removes a motivation for
removing a time bomb from the code. In the real-life code from
which this example was taken, macros obscured the class
beginning and end with their m_begin and m_end guards. The code
maintainer needed to add a bool and an int field to the class,
which would've broken the code if maintainer had not first
carefully analyzed it or just had good luck in realizing what
was happening. The next programmer to touch that code may not
be so lucky or smart.

I'll search the standard for a relevant reference, then I'll
write a unit test and refactor this sucker.

Because the compiler knows all the offsets of the members, is
there any chance a near future version of C++ will offer
iterators to access the typeinfo structures and offsets of each
member of a class?

/Glen

[red floyd]

Glen Dayton wrote:

While looking at some old code I ran across a snippet that increments a
pointer to access different members of a structure:
...

struct Badness {
std::string m_begin;
std::string m_param1;
std::string m_param2;
std::string m_end;

Badness();
};
...
Badness badness;

std::string *it = &badness.m_begi n;

for (++it; it != &badness.m_e nd; ++it) {
std::cout << it->c_str() << std::endl;
}

I think if you want to avoid UB, you're toast, unless you want to use
pointers to members. Maybe something like this? Note: m_members should
probably be some sort of static.

struct Badness {
std::string m_begin;
std::string m_param1;
std::string m_param2;
std::string m_end;

typedef std::vector<std ::string Badness::*membe r_vec_t;
member_vec_t m_members;
Badness()
{
m_members.push_ back(&Badness:: m_begin);
m_members.push_ back(&Badness:: m_param1);
m_members.push_ back(&Badness:: m_param2);
m_members.push_ back(&Badness:: m_end);
}

};
// ...
Badness badness;
for (Badness::membe r_vec_t it = badness.m_membe rs.begin();
it != badness.m_membe rs.end();
++it)
std::cout << badness.*(*it) << std::endl;

[Pete Becker]

Glen Dayton wrote:

Pete Becker wrote:

>Glen Dayton wrote:

>>>
Looking wrong and evil, though, are not rational arguments that would
allow me to convince my technical lead that this code needs to be
changed sooner than later.

If it works, it works. The behavior isn't defined by the C++ standard,
but I haven't run into a compiler where it won't do just what it ooks
like it does.

That it is undefined by the standard is one problem. It means at a
future date the code will stop working.

No, it means that at a future date the code might stop working. That's
not to say that it's a good way to write code, but hyperventilatin g is
not a good way to start a design.

--

-- Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com)
Author of "The Standard C++ Library Extensions: a Tutorial and
Reference." (www.petebecker.com/tr1book)

[Gavin Deane]

Pete Becker wrote:

Glen Dayton wrote:

Pete Becker wrote:

If it works, it works. The behavior isn't defined by the C++ standard,
but I haven't run into a compiler where it won't do just what it ooks
like it does.

That it is undefined by the standard is one problem. It means at a
future date the code will stop working.

No, it means that at a future date the code might stop working. That's
not to say that it's a good way to write code, but hyperventilatin g is
not a good way to start a design.

I think there is quite a difference between intending to use formally
undefined behaviour in a new design and changing existing code that
works when you discover it relies on formally undefined behaviour.

Personally, for a new design my initial position would be to avoid any
code for which the behaviour is undefined. With an existing code base,
if I was satisfied that the behaviour on my particular compiler and
platform was as intended, my initial position would be if it ain't
broken, don't fix it. Of course, formally the code is broken by
definition. But in practice I would take into account whether I
expected to be changing compilers any time soon, and how much
regression test coverage I had of the code before deciding whether to
change it.

Gavin Deane

[Salt_Peter]

Glen Dayton wrote:

While looking at some old code I ran across a snippet that
increments a pointer to access different members of a structure:
...

struct Badness {
std::string m_begin;
std::string m_param1;
std::string m_param2;
std::string m_end;

Badness();
};
...
Badness badness;

std::string *it = &badness.m_begi n;

for (++it; it != &badness.m_e nd; ++it) {
std::cout << it->c_str() << std::endl;
}

It just looks wrong. It depends on all of the members of class
or structure to be consistent so its bad from a maintenance
standpoint. It seems evil to use a pointer beyond the specific
object for which it is defined.

Looking wrong and evil, though, are not rational arguments that
would allow me to convince my technical lead that this code
needs to be changed sooner than later.

Anyone have good talking points?

/Glen

Loose the pointers.
Can you not do something like this? Note: you must keep the
std::vector< std::string container at the top of the struct in order
to guarentee that it is initialized before the references.

#include <iostream>
#include <ostream>
#include <string>
#include <vector>
#include <iterator>

struct Badness {
private:
std::vector< std::string vs;
public:
std::string& m_begin;
std::string& m_param1;
std::string& m_param2;
std::string& m_end;
typedef std::vector< std::string >::iterator baditerator;
typedef std::vector< std::string >::const_iterat or const_baditerat or;

Badness() : vs(4),
m_begin(vs[0]), m_param1(vs[1]),
m_param2(vs[2]), m_end(vs[3])
{ }
baditerator begin() { return vs.begin(); }
baditerator end() { return vs.end(); }

// friend
friend std::ostream&
operator<<(std: :ostream& os, const Badness& r_bad)
{
std::copy( r_bad.vs.begin( ),
r_bad.vs.end(),
std::ostream_it erator< std::string >(os, "\n") );
return os;
}
};

int main()
{
Badness badness;
badness.m_begin = "string m_begin";
badness.m_param 1 = "string m_param1";
badness.m_param 2 = "string m_param2";
badness.m_end = "string m_end";

for (Badness::badit erator iter = badness.begin() ;
iter != badness.end();
++iter)
{
std::cout << *iter << std::endl;
}

std::cout << "\n using op<<...\n";
std::cout << badness;
}

[Pete Becker]

Gavin Deane wrote:

Of course, formally the code is broken by
definition.

For a somewhat artificial definition of "broken." If it does what it's
supposed to do most people wouldn't consider it broken.

But in practice I would take into account whether I
expected to be changing compilers any time soon, and how much
regression test coverage I had of the code before deciding whether to
change it.

Portability is not an absolute. It's about how hard it is to move to a
different platform. Code that's well formed according to the language
definition isn't automatically easier to port than code that isn't.
Compilers vary.

--

-- Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com)
Author of "The Standard C++ Library Extensions: a Tutorial and
Reference." (www.petebecker.com/tr1book)

[Gavin Deane]

Pete Becker wrote:

Gavin Deane wrote:

Of course, formally the code is broken by
definition.

For a somewhat artificial definition of "broken." If it does what it's
supposed to do most people wouldn't consider it broken.

It does what it's supposed to do, but how do I know that? Where is the
specification that defines what behavior that code should have?

If I am relying on undefined behaviour, the simple fact that there is
nowhere I can go to read documentation that explains what the behaviour
should be makes me uncomfortable as an engineer. Operating outside the
world of specification is moving away from sound engineering principles
and towards voodoo. I would suggest that a software engineer should
only do that with good reason.

Gavin Deane