I'm doing a code review and noticed some code that's not well-written,
and I've forgotten the reason why.
Here's a brief excerpt with names changed to protect the innocent, er,
the IP:
class MyPoint
{
..
:
protected:
int x;
int y;
}
class MyRect
{
..
:
MyPoint & MyRect::TopLeft()
{
return *( ( MyPoint * ) this );
}
MyPoint & MyRect::BottomRight()
{
return *( ( MyPoint * ) this+1 );
}
..
:
protected:
LONG left;
LONG top;
LONG right;
LONG bottom;
}
17  1461  ci***********@gmail.com schrieb:
I'm doing a code review and noticed some code that's not well-written,
and I've forgotten the reason why.
Here's a brief excerpt with names changed to protect the innocent, er,
the IP:
class MyPoint
{
.
:
protected:
int x;
int y;
}
class MyRect
{
.
:
MyPoint & MyRect::TopLeft()
{
return *( ( MyPoint * ) this );
}
MyPoint & MyRect::BottomRight()
{
return *( ( MyPoint * ) this+1 );
}
.
:
protected:
LONG left;
LONG top;
LONG right;
LONG bottom;
}
Curious Optimization???
(Note: this is different from Curious Templates) ci***********@gmail.com wrote:
I'm doing a code review and noticed some code that's not well-written,
and I've forgotten the reason why.
Here's a brief excerpt with names changed to protect the innocent, er,
the IP:
class MyPoint
{
.
>>
protected:
int x;
int y;
}
;
>
class MyRect
{
.
>>
MyPoint & MyRect::TopLeft()
{
return *( ( MyPoint * ) this );
}
MyPoint & MyRect::BottomRight()
{
return *( ( MyPoint * ) this+1 );
}
.
>>
protected:
LONG left;
'LONG' is undefined.
LONG top;
LONG right;
LONG bottom;
}
;
Bad? I don't know. This code has undefined behaviour as far as C++
is concerned. Whether it's bad or not is for you to decide...
V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask ci***********@gmail.com wrote:
I'm doing a code review and noticed some code that's not well-written,
and I've forgotten the reason why.
Here's a brief excerpt with names changed to protect the innocent, er,
the IP:
class MyPoint
{
.
:
protected:
int x;
int y;
}
class MyRect
{
.
:
MyPoint & MyRect::TopLeft()
{
return *( ( MyPoint * ) this );
}
MyPoint & MyRect::BottomRight()
{
return *( ( MyPoint * ) this+1 );
}
These are reinterpret casts. Dereferencing such a casted pointer is,
I'm pretty sure, totally undefined. Not only that but the reinterpret
cast of this+1 is almost certainly not what is wanted. They probably
thought ((MyPoint*)this) + 1 but this is even less predictable. This
is gross in so many ways.
In short, the behavior of this class is totally unpredictable. The
fact that the standard doesn't define a LONG is quite beside the
point...you don't need a definition to see the problems here.
MyRect should contain points that can be returned by these functions.
The code you are reviewing is unnecissarily terse and its nature is
undefined even if we can guess at the goal to some degree.
.
:
protected:
LONG left;
LONG top;
LONG right;
LONG bottom;
}
<ci***********@gmail.comwrote in message
news:11*********************@m79g2000cwm.googlegro ups.com...
I'm doing a code review and noticed some code that's not well-written,
and I've forgotten the reason why.
If you've forgotten why, then what made you decide it was bad? Sounds more
like a homework assignment to me...
-H
Howard wrote:
<ci***********@gmail.comwrote in message
news:11*********************@m79g2000cwm.googlegro ups.com...
>I'm doing a code review and noticed some code that's not well-written,
and I've forgotten the reason why.
If you've forgotten why, then what made you decide it was bad? Sounds more
like a homework assignment to me...
Her question seems sincere to me and hardly looks like homework. I
don't think students are likely to recast their questions in the context
of code reviews, nor are they likely to mention IP as part of their back
story.
Noah Roberts <ro**********@gmail.comwrote:
>ci***********@gmail.com wrote:
>I'm doing a code review and noticed some code that's not well-written,
and I've forgotten the reason why.
Here's a brief excerpt with names changed to protect the innocent, er,
the IP:
class MyPoint
{
.
:
protected:
int x;
MyPoint & MyRect::TopLeft()
{
return *( ( MyPoint * ) this );
}
MyPoint & MyRect::BottomRight()
{
return *( ( MyPoint * ) this+1 );
}
>These are reinterpret casts. Dereferencing such a casted pointer is,
I'm pretty sure, totally undefined. Not only that but the reinterpret
cast of this+1 is almost certainly not what is wanted.
I don't see a cast of (this + 1) in the above code.
They probably thought ((MyPoint*)this) + 1
That's what they wrote.
Steve
Noah Roberts wrote:
ci***********@gmail.com wrote:
>I'm doing a code review and noticed some code that's not well-written,
and I've forgotten the reason why.
Here's a brief excerpt with names changed to protect the innocent, er,
the IP:
class MyPoint
{
.
:
protected:
int x;
int y;
}
class MyRect
{
.
:
MyPoint & MyRect::TopLeft()
{
return *( ( MyPoint * ) this );
}
MyPoint & MyRect::BottomRight()
{
return *( ( MyPoint * ) this+1 );
}
These are reinterpret casts. Dereferencing such a casted pointer is,
I'm pretty sure, totally undefined. Not only that but the reinterpret
cast of this+1 is almost certainly not what is wanted. They probably
thought ((MyPoint*)this) + 1 but this is even less predictable. This
is gross in so many ways.
Just a nit to pick:
(MyPoint*)this + 1
is the same as:
((MyPoint*)this) + 1
--
Clark S. Cox III cl*******@gmail.com
On 23 Aug 2006 13:57:17 -0700 in comp.lang.c++, ci***********@gmail.com wrote,
>I'm doing a code review and noticed some code that's not well-written,
and I've forgotten the reason why.
The reason it's bad is that it throws away most of the potential
help and protection that C++ can give you with regard to accessing
the members of your class. By using a reinterpret cast (in the
form of a C cast) you are promising the compiler that you know what
you are doing and everything will be OK. If you make the slightest
slip, then everything will not be OK. If you write type-safe C++
without casts, the compiler gives you some assurances that things
will be OK instead of the other way around.
For example, you have class MyPoint with an x member coming before
the y member. Later you have a class MyRect which ought to have two
MyPoint members, corresponding to upper left and lower right.
Instead MyRect has four LONG members that by PURE COINCIDENCE might
possibly mimic the layout of two MyPoints. If you're lucky. If
"LONG" happens to be the same size as "int" and so forth. If nobody
decides to change one of them and forgets to change the other.
Nobody looking at class MyRect by itself has the FAINTEST HINT that
the order of those members, or anything else, depends on MyPoint.
Nobody looking at MyPoint can tell that MyRect depends on it. The
dependency is hidden in the cracks between them.
posted:
I'm doing a code review and noticed some code that's not well-written,
and I've forgotten the reason why.
We don't have enough context -- I haven't got a clue what the code is
doing.
Here's a legitimate example though:
struct Coords {
int x, y;
};
struct Location {
int x,y;
char place_name[128];
Coords &GetCoords()
{
return (Coords&)*this;
}
};
int main()
{
Location london;
london.GetCoords().x = 5;
}
The first member of a POD is guaranteed to have the same address as the POD
object itself. POD structs which have a common initial sequence can be
accessed... blah blah blah (can't remember the exact quote).
--
Frederick Gotham
Frederick Gotham wrote:
posted:
>I'm doing a code review and noticed some code that's not
well-written, and I've forgotten the reason why.
We don't have enough context -- I haven't got a clue what the code is
doing.
Here's a legitimate example though:
struct Coords {
int x, y;
};
struct Location {
int x,y;
char place_name[128];
Coords &GetCoords()
{
return (Coords&)*this;
}
};
int main()
{
Location london;
london.GetCoords().x = 5;
}
The first member of a POD is guaranteed to have the same address as
the POD object itself. POD structs which have a common initial
sequence can be accessed... blah blah blah (can't remember the exact
quote).
Legitimate? Maybe. Sensible? I don't think so. Why would this be
any better than, say,
struct Coords {
int x, y;
};
struct Location {
Coords c;
...
?
V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask
Victor Bazarov posted:
Legitimate? Maybe. Sensible? I don't think so. Why would this be
any better than, say,
struct Coords {
int x, y;
};
struct Location {
Coords c;
...
Of course, that would be better. However, I wonder if there's a funky reason
why the programmer chose not to do that (other than lack of intelligence).
--
Frederick Gotham
Howard wrote:
If you've forgotten why, then what made you decide it was bad? Sounds more
like a homework assignment to me...
Nope! My last homework assignment was back in 1994. I've been off in
C for a long time, then returned to C++ via MFC. I'm a bit rusty.
I appreciate everyone's comments; it's been a rough week.
Cindy
I'll ask the programmer tomorrow when we have our review meeting. I
really wasn't clear on why, either.
Cindy
Frederick Gotham wrote:
Victor Bazarov posted:
Legitimate? Maybe. Sensible? I don't think so. Why would this be
any better than, say,
struct Coords {
int x, y;
};
struct Location {
Coords c;
...
Of course, that would be better. However, I wonder if there's a funky reason
why the programmer chose not to do that (other than lack of intelligence).
--
Frederick Gotham
Thank you for the detailed reply!
I didn't just want to say "it's bad code". The maintainability thing
was obvious, but the point it throws away most of the benefits of C++
was not (to me, yesterday, anyway).
Cindy
David Harmon wrote:
On 23 Aug 2006 13:57:17 -0700 in comp.lang.c++,
ci***********@gmail.com wrote,
I'm doing a code review and noticed some code that's not well-written,
and I've forgotten the reason why.
The reason it's bad is that it throws away most of the potential
help and protection that C++ can give you with regard to accessing
the members of your class. By using a reinterpret cast (in the
form of a C cast) you are promising the compiler that you know what
you are doing and everything will be OK. If you make the slightest
slip, then everything will not be OK. If you write type-safe C++
without casts, the compiler gives you some assurances that things
will be OK instead of the other way around.
For example, you have class MyPoint with an x member coming before
the y member. Later you have a class MyRect which ought to have two
MyPoint members, corresponding to upper left and lower right.
Instead MyRect has four LONG members that by PURE COINCIDENCE might
possibly mimic the layout of two MyPoints. If you're lucky. If
"LONG" happens to be the same size as "int" and so forth. If nobody
decides to change one of them and forgets to change the other.
Nobody looking at class MyRect by itself has the FAINTEST HINT that
the order of those members, or anything else, depends on MyPoint.
Nobody looking at MyPoint can tell that MyRect depends on it. The
dependency is hidden in the cracks between them.
Cindy posted:
I've been off in
C for a long time, then returned to C++ via MFC.
What a disgusting conduit through which to return to C++.
Word of advice: Do the exact opposite of everything that Microsoft does --
they have VERY poor programming practises.
--
Frederick Gotham
"Frederick Gotham" <fg*******@SPAM.comwrote in message
news:ce*******************@news.indigo.ie...
Cindy posted:
>I've been off in
C for a long time, then returned to C++ via MFC.
What a disgusting conduit through which to return to C++.
Word of advice: Do the exact opposite of everything that Microsoft does --
they have VERY poor programming practises.
Geez... how about keeping your hatred of Microsoft out of this?
-H This thread has been closed and replies have been disabled. Please start a new discussion. Similar topics
by: Mike |
last post by:
I am using MS-Access as a front end for my MS-SQL DB. I have a sql view that
uses the following:
SELECT TOP 100 PERCENT RECID, PATNUMBER AS , SVCCODE AS , QTY, PROF_CHRGS AS , AMOUNT,...
|
by: Tim |
last post by:
I'm trying to co-erce a __gc array of Byte to a __nogc pointer to char to
pass to a native function call in a bit of managed c++ code like this:
Byte field __gc = dynamic_cast<Byte...
|
by: John Howard |
last post by:
Making the following call to a local MSAccess database works fine:
Sub Session_Start(ByVal sender As Object, ByVal e As EventArgs)
Dim intRows As Integer
Dim strSQL As String
Dim ds As New...
|
by: PK9 |
last post by:
I am looking for assistance in pinpointing the cause of the following
exception. I am getting a "Specified Cast is not valid" exception on my
page. I am trying to populate a datagrid. One of my...
|
by: Reza Nabi |
last post by:
Bakground: I have a webform (LoadCtl.aspx) which loads the user control to a
placeholder dynamically based on the ctlName querystring passed in the URL.
Webform (LoadCtl.aspx) also passes a...
|
by: Mike Cooper |
last post by:
There is something about inherited classes I evidently don't know...
I wrote the following class:
Class Class1
inherits System.Windows.Forms.DataGridTextBoxColumn
End Class
There is...
|
by: Chris Thunell |
last post by:
I'm trying to loop through an exchange public folder contact list, get some
information out of each item, and then put it into a vb.net datatable. I
run though the code and all works fine until i...
|
by: Charles Law |
last post by:
I have the following lines
Dim t As Type = GetType(MyType)
Dim serialiser As New XmlSerializer(t)
I want to be able to do the following with a FileStream fs
Dim instance As MyType
...
|
by: Frederick Gotham |
last post by:
Let's assume that we're working on the following system:
CHAR_BIT == 8
sizeof( char* ) == 4 (i.e. 32-Bit)
Furthermore, lets assume that the memory addresses are distributed as
follows:
...
|
by: Dinsdale |
last post by:
I am trying to determine what the current cast of an object is.
Essentially, if I use GetType, it always returns the original type of
the object created but I want to know what the current cast of...
|
by: aa123db |
last post by:
Variable and constants
Use var or let for variables and const fror constants.
Var foo ='bar';
Let foo ='bar';const baz ='bar';
Functions
function $name$ ($parameters$) {
}
...
|
by: ryjfgjl |
last post by:
If we have dozens or hundreds of excel to import into the database, if we use the excel import function provided by database editors such as navicat, it will be extremely tedious and time-consuming...
|
by: ryjfgjl |
last post by:
In our work, we often receive Excel tables with data in the same format. If we want to analyze these data, it can be difficult to analyze them because the data is spread across multiple Excel files...
|
by: emmanuelkatto |
last post by:
Hi All, I am Emmanuel katto from Uganda. I want to ask what challenges you've faced while migrating a website to cloud.
Please let me know.
Thanks!
Emmanuel
|
by: BarryA |
last post by:
What are the essential steps and strategies outlined in the Data Structures and Algorithms (DSA) roadmap for aspiring data scientists? How can individuals effectively utilize this roadmap to progress...
|
by: nemocccc |
last post by:
hello, everyone, I want to develop a software for my android phone for daily needs, any suggestions?
|
by: Sonnysonu |
last post by:
This is the data of csv file
1 2 3
1 2 3
1 2 3
1 2 3
2 3
2 3
3
the lengths should be different i have to store the data by column-wise with in the specific length.
suppose the i have to...
|
by: Oralloy |
last post by:
Hello folks,
I am unable to find appropriate documentation on the type promotion of bit-fields when using the generalised comparison operator "<=>".
The problem is that using the GNU compilers,...
|
by: jinu1996 |
last post by:
In today's digital age, having a compelling online presence is paramount for businesses aiming to thrive in a competitive landscape. At the heart of this digital strategy lies an intricately woven...
| |
