Reference question
Larry....@gmail.com
some code I don't understand.
static struct ct {
char ns, fl;
short sf;
struct st
{
char sy, fq;
ct* n;
} __attribute__ ((packed)) * sta;
st& os() const
{
return (st&) sf;
}
} __attribute__ ((packed)) *mc;
My confusion is about the os method. It appears to me that it returns
a reference to the st struct. But how does returning sf become a
reference to st?
TIA
-larry
Victor Bazarov
Beats me. They are probably trying to rely on some obscure memory
layout compatibility that exists between the beginning of the 'st'
struct and a 'short'. Generally speaking a cast like that is a VERY BAD
IDEA(tm).
V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask
Michał 'Khorne' Rzechonek
> > short sf;
>
> > struct st
> > {
> > char sy, fq;
> > ct* n;
> > } __attribute__ ((packed)) * sta;
>
> > st& os() const
> > {
> > return (st&) sf;
> > }
> Beats me. They are probably trying to rely on some obscure memory
> layout compatibility that exists between the beginning of the 'st'
> struct and a 'short'. Generally speaking a cast like that is a VERY BAD
> IDEA(tm).
I concur. Things like this are common in C and other code fiddling
with
memory on byte level. Atrribute "packed" is a good way to map some
buffer
to well defined fields.
I would add static asserion checking for sizeof(short) being no less
than
sizeof(st), though.
--
Khorne
Christopher
This code was written by one of those people that would get me fired
for arguing with them too much.
I would go to thier desk and tell them to stop writing garbage. I mean
everyone should know what a ct is and what its used for with our ESP
powers right?
Same for an ns, fl, or st right?
The names make it so obvious what this code is for!
Jeff Schwab
> On Jan 28, 9:35 am, "Larry.Mart...@gmail.com"
>> static struct ct {
>> char ns, fl;
>> short sf;
>>
>> struct st
>> {
>> char sy, fq;
>> ct* n;
>> } __attribute__ ((packed)) * sta;
>>
>> st& os() const
>> {
>> return (st&) sf;
>> }
>>
>> } __attribute__ ((packed)) *mc;
>>
> everyone should know what a ct is and what its used for with our ESP
> powers right?
> Same for an ns, fl, or st right?
>
> The names make it so obvious what this code is for!
This is one my own personal peeves. It blows my mind how differently
some people feel about abbreviations in code. "It's clear enough to
*me*, and to all these other people who've spent the last several years
immersed in this code base; why isn't it clear to *you*? You're just
being pedantic. Jeez, it seems like each new candidate we hire takes
longer than the last to come up to speed. The quality of candidates
must be dropping."
Larry....@gmail.com
I know it's crappy code - it's some of the worst code I've ever seen.
I didn't write it, I inherited it - and I have to support and modify
it. I plan to completely rewrite it - once I understand it and have
the time to work on it.
Chris Gordon-Smith
This reminds me of the quote:-
"Programs must be written for people to read, and only incidentally for
machines to execute." I think may be from Abelson and Sussman.
I agree of course that names such as "st" are an outrage. I suppose that it
is possible to go too far. When I look at code I have written in the past I
sometimes think I might have opted for something a little more concise. For
example, I used to write 'Molecule_Type', but now I write 'MolType'. I
probably won't go further to 'MolTp', and I hope I never resort to the
awful 'mt'.
Another thing to add about the above code (apart from the fact that it
probably should never have been written), is that if it must be written,
then it should be commented.
--
Chris Gordon-Smith
London
www.simsoup.info
Jeff Schwab
I feel your pain. Good luck.
Jeff Schwab
> I used to write 'Molecule_Type', but now I write 'MolType'.
Why? Doesn't your editor support auto-completion? Don't get me wrong;
if it's your code, you get to choose the names. Those kinds of
abbreviations can make life a lot less pleasant, though, for anyone else
who has to maintain the code.
Pete Becker
Reading code with long names can be frustrating, especially if you have
to scroll the screen to see the entire line, or if lines have to be
broken at odd places in order to fit on a screen. Operators, being one
or two characters, can be hard to find in a welter of words.
--
Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com) Author of "The
Standard C++ Library Extensions: a Tutorial and Reference
(www.petebecker.com/tr1book)
Jeff Schwab
> On 2009-01-28 18:43:40 -0500, Jeff Schwab <je...@schwabcenter.com> said:
>
>> Chris Gordon-Smith wrote:
>>
>>> I used to write 'Molecule_Type', but now I write 'MolType'.
>>
>> Why? Doesn't your editor support auto-completion? Don't get me
>> wrong; if it's your code, you get to choose the names. Those kinds of
>> abbreviations can make life a lot less pleasant, though, for anyone
>> else who has to maintain the code.
>
> Reading code with long names can be frustrating, especially if you have
> to scroll the screen to see the entire line, or if lines have to be
> broken at odd places in order to fit on a screen. Operators, being one
> or two characters, can be hard to find in a welter of words.
That's a sign that it's past time to refactor. The solution is not to
start abbreviating. If you have a bunch of names that all start with
Molecule, there probably ought to be a molecule namespace. Given crappy
code, the solution is not to make it crappier.
James Kanze
> Larry.Mart...@gmail.com wrote:
> > I have only been working with C++ for 3 years, and I've come across
> > some code I don't understand.
> > static struct ct {
> > char ns, fl;
> > short sf;
> > struct st
> > {
> > char sy, fq;
> > ct* n;
> > } __attribute__ ((packed)) * sta;
> > st& os() const
> > {
> > return (st&) sf;
> > }
> > } __attribute__ ((packed)) *mc;
> > My confusion is about the os method. It appears to me that
> > it returns a reference to the st struct. But how does
> > returning sf become a reference to st?
> Beats me. They are probably trying to rely on some obscure
> memory layout compatibility that exists between the beginning
> of the 'st' struct and a 'short'. Generally speaking a cast
> like that is a VERY BAD IDEA(tm).
The key, I think, is the presence of __attribute__((packed)) all
over the place. This is obviously very platform dependent code
(and probably very low level); without knowing the exact
platform, and the context in which it occurs, we can't say any
more (and knowing all that, of course, the response would be off
topic here). Generally speaking, as you say, the cast like that
is a very bad idea, but I can imagine cases in the kernel of an
OS where it might be appropriate.
(One thing does worry me a bit---the __attribute__ is g++
syntax, so I would assume g++. And g++ generates bad code when
such casts are used, on the grounds that the results are
undefined behavior. So the "implementation defined" behavior of
what he is doing is "undefined behavior".)
--
James Kanze (GABI Software) email:james...@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
Pete Becker
I don't see how changing Molecule_type to Molecule::Type relieves the problem.
Jeff Schwab
> I don't see how changing Molecule_type to Molecule::Type relieves the
> problem.
Within the set of code that works with Molecules, you only have to call
it Type. (I'm not in love with that name, either.) This avoids the
long names (which neither you nor I seem to like very much), without the
need for abbreviating the English words used in those names.
using Molecule::Type;
Or, better:
using chemistry::molecule;
Or, if the code that works with the type in question is sufficiently
generic (unlikely in the case of molecules), pass it as a template argument.
When a single piece of code has to reach into many different namespaces,
or use codes with many different prefixes (e.g. Mol), one suspects
that the code lacks appropriate domain abstractions. I realize that
sounds foofy, but like it or not, maintaining non-trivial code always
requires some understanding of the application domain, and it's very
helpful if the code has been factored to fit that domain.
There are lots of "greedy algorithms" for making code slightly less
verbose, without refactoring: Abbreviating identifiers, using two-space
indentation, cramming each inline function definition or if-statement
onto a single line, etc. I am convinced that these things just make the
code harder to maintain in the long run. Code that has to last, and
grow, and change, looks very different from the kind of code that wins
prizes on TopCoder.
Pete Becker
> Pete Becker wrote:
>
>> I don't see how changing Molecule_type to Molecule::Type relieves the problem.
>
> Within the set of code that works with Molecules, you only have to call
> it Type. (I'm not in love with that name, either.) This avoids the
> long names (which neither you nor I seem to like very much), without
> the need for abbreviating the English words used in those names.
Right. Except when there's more than one namespace that has a name
Type, in which case you're right back to qualified names.
The problem with long names in inherent: they're long. And when you
stack several long names together you end up with an abomination.
>
> using Molecule::Type;
>
> Or, better:
>
> using chemistry::molecule;
>
> Or, if the code that works with the type in question is sufficiently
> generic (unlikely in the case of molecules), pass it as a template
> argument.
>
> When a single piece of code has to reach into many different
> namespaces, or use codes with many different prefixes (e.g. Mol), one
> suspects that the code lacks appropriate domain abstractions. I
> realize that sounds foofy, but like it or not, maintaining non-trivial
> code always requires some understanding of the application domain, and
> it's very helpful if the code has been factored to fit that domain.
How would you refactor a name like time_since_epoch(), which takes up
20% of my line every time I have to use it? Oh, and I can't change it:
it's part of the interface to a library that I don't control.
>
> There are lots of "greedy algorithms" for making code slightly less
> verbose, without refactoring: Abbreviating identifiers, using two-space
> indentation, cramming each inline function definition or if-statement
> onto a single line, etc. I am convinced that these things just make
> the code harder to maintain in the long run. Code that has to last,
> and grow, and change, looks very different from the kind of code that
> wins prizes on TopCoder.
I don't know what "TopCoder" is. I know that when I write code that
uses libraries whose interfaces were designed with "use long names" the
result is unreadable.
Jeff Schwab
> How would you refactor a name like time_since_epoch(), which takes up
> 20% of my line every time I have to use it?
Why does 20% concern you? Just how many names per line are you trying
to achieve?
> Oh, and I can't change it:
> it's part of the interface to a library that I don't control.
Boo hoo. Nobody ever taught you about inline wrapper functions? If the
meaning is already clear from context, use a simple name like "time".
Are you really claiming that you would have preferred an interface with
names like eptime, or timsinep?
Every time you add a dependency on some external library, do you really
just start using the library directly, from all over your code base,
without any insulation or compile-time wrapper layer? That seems...
Brittle.
> I know that when I write code that uses
> libraries whose interfaces were designed with "use long names" the
> result is unreadable.
Plenty of useful libraries have horrid interfaces, and "long names" are
hardly the worst part. The solution I tend to use is to build my own
little bridge layer that exposes the parts of the library I actually
use. It doesn't actually take that long, relative to the time saved in
not letting the poorly designed interface infect the rest of my code.
Pete Becker
> Pete Becker wrote:
>
>> How would you refactor a name like time_since_epoch(), which takes up
>> 20% of my line every time I have to use it?
>
> Why does 20% concern you? Just how many names per line are you trying
> to achieve?
As many as are needed.
sumOfTheThingsICareAbout = firstThingICareAbout
+ secondThingICareAbout
+ thirdThingICareAbout;
sum = first + second + third;
sum = a + b + c;
In that last one, the numbering is implicit in the names. No need to be
longwinded.
Have you ever tried to read Java code? The long-names-are-good
designers had a field day there, and the result is often quite hard to
read.
>
>> Oh, and I can't change it: it's part of the interface to a library that
>> I don't control.
>
> Boo hoo. Nobody ever taught you about inline wrapper functions?
Oh, good idea: I'll just write a new class that provides the interface
that I prefer. Everyone who maintains my code can then learn my
interface as well as the library's interface.
> If the meaning is already clear from context, use a simple name like
> "time". Are you really claiming that you would have preferred an
> interface with names like eptime, or timsinep?
No, certainly not. The fact that you can make up bad names doesn't mean
that names have to be too long.
>
> Every time you add a dependency on some external library, do you really
> just start using the library directly, from all over your code base,
> without any insulation or compile-time wrapper layer? That seems...
> Brittle.
Sigh.
>
>> I know that when I write code that uses libraries whose interfaces were
>> designed with "use long names" the result is unreadable.
>
> Plenty of useful libraries have horrid interfaces, and "long names" are
> hardly the worst part. The solution I tend to use is to build my own
> little bridge layer that exposes the parts of the library I actually
> use. It doesn't actually take that long, relative to the time saved in
> not letting the poorly designed interface infect the rest of my code.
If it's strictly your code, knock yourself out. If you're part of a
project there's the cost of everyone else, including future
maintainers, having to learn an additional interface to something they
may already know well.
Jeff Schwab
> On 2009-01-29 12:08:39 -0500, Jeff Schwab <je...@schwabcenter.com> said:
>
>> Pete Becker wrote:
>>
>>> How would you refactor a name like time_since_epoch(), which takes up
>>> 20% of my line every time I have to use it?
>>
>> Why does 20% concern you? Just how many names per line are you trying
>> to achieve?
>
> As many as are needed.
>
> sumOfTheThingsICareAbout = firstThingICareAbout
> + secondThingICareAbout
> + thirdThingICareAbout;
(1) That's only two names per line, with a single statement spanning
three lines.
(2) No individual piece of code should have to refer to lots of
different long names. Nobody here is defending that.
> Have you ever tried to read Java code? The long-names-are-good designers
> had a field day there, and the result is often quite hard to read.
Java's a different language with very abstraction mechanisms. In Java,
you haven't even got typedef.
>> Boo hoo. Nobody ever taught you about inline wrapper functions?
>
> Oh, good idea: I'll just write a new class that provides the interface
> that I prefer. Everyone who maintains my code can then learn my
> interface as well as the library's interface.
They're going to have to learn your code and conventions anyway. It's
the application domain, not the library, that seems (in my finite
experience) to be the biggest hurdle for developers new to a particular
code base.
>> If the meaning is already clear from context, use a simple name like
>> "time". Are you really claiming that you would have preferred an
>> interface with names like eptime, or timsinep?
>
> No, certainly not. The fact that you can make up bad names doesn't mean
> that names have to be too long.
Right. We agree. In my opinion, MolType is a bad name, and
chemistry::molecule is a better one. If your code is even reasonably
modular, you don't have to type chemistry::molecule all over the place.
>> Every time you add a dependency on some external library, do you
>> really just start using the library directly, from all over your code
>> base, without any insulation or compile-time wrapper layer? That
>> seems... Brittle.
>
> Sigh.
OK... Sigh, too.
>>> I know that when I write code that uses libraries whose interfaces
>>> were designed with "use long names" the result is unreadable.
>>
>> Plenty of useful libraries have horrid interfaces, and "long names"
>> are hardly the worst part. The solution I tend to use is to build my
>> own little bridge layer that exposes the parts of the library I
>> actually use. It doesn't actually take that long, relative to the
>> time saved in not letting the poorly designed interface infect the
>> rest of my code.
>
> If it's strictly your code, knock yourself out. If you're part of a
> project there's the cost of everyone else, including future maintainers,
> having to learn an additional interface to something they may already
> know well.
Let me get this straight: We should tie our code to crappy library
interfaces, because some developers are already familiar with them?
Give me a break. If your application is doing anything interesting,
it's going to require layers on top of those libraries, anyway.
What I said was a pet peeve was inappropriate abbreviation. You seem to
have taken this as some kind of defense of long names. I'm not sure how
to make this any clearer, so unless you have something new to add, I
think we're through here.
Pete Becker
> Pete Becker wrote:
>> On 2009-01-29 12:08:39 -0500, Jeff Schwab <je...@schwabcenter.com> said:
>>
>>> Pete Becker wrote:
>>>
>>>> How would you refactor a name like time_since_epoch(), which takes up
>>>> 20% of my line every time I have to use it?
>>>
>>> Why does 20% concern you? Just how many names per line are you trying
>>> to achieve?
>>
>> As many as are needed.
>>
>> sumOfTheThingsICareAbout = firstThingICareAbout
>> + secondThingICareAbout
>> + thirdThingICareAbout;
>
> (1) That's only two names per line, with a single statement spanning
> three lines.
Sigh. Yes, because the names were, cumulatively, too long to fit on a
single line. Since what should be one line is now three, there are two
fewer lines available to see the rest of the source code. That makes it
harder to get an overview of the code.
>
> (2) No individual piece of code should have to refer to lots of
> different long names. Nobody here is defending that.
>
>> Have you ever tried to read Java code? The long-names-are-good
>> designers had a field day there, and the result is often quite hard to
>> read.
>
> Java's a different language with very abstraction mechanisms. In Java,
> you haven't even got typedef.
Sigh. Nevertheless, it shows what happens when long names take over.
>
>>> Boo hoo. Nobody ever taught you about inline wrapper functions?
>>
>> Oh, good idea: I'll just write a new class that provides the interface
>> that I prefer. Everyone who maintains my code can then learn my
>> interface as well as the library's interface.
>
> They're going to have to learn your code and conventions anyway.
Sigh. Sure: since they have to learn some stuff, why not dump even more in?
> It's the application domain, not the library, that seems (in my
> finite experience) to be the biggest hurdle for developers new to a
> particular code base.
>
>>> If the meaning is already clear from context, use a simple name like
>>> "time". Are you really claiming that you would have preferred an
>>> interface with names like eptime, or timsinep?
>>
>> No, certainly not. The fact that you can make up bad names doesn't mean
>> that names have to be too long.
>
> Right. We agree. In my opinion, MolType is a bad name, and
> chemistry::molecule is a better one. If your code is even reasonably
> modular, you don't have to type chemistry::molecule all over the place.
Hmm, interesting: the names I refered to were, obviously, "eptime" and
"timsinep". But somehow you ended up talking about MolType, which
neither of us had mentioned. Bait and switch, anyone?
It's quite obvious that we're through here.
Jeff Schwab
> Hmm, interesting: the names I refered to were, obviously, "eptime" and
> "timsinep". But somehow you ended up talking about MolType, which
> neither of us had mentioned. Bait and switch, anyone?
That's the name I didn't like. You replied to me, apparently defending
it. One of us may be "baiting and switching," but it isn't me.
Pete Becker
I never said anything one way or the other about MolType. But that's
not apparent, since you've snipped the context here.
Chris Gordon-Smith
> Pete Becker wrote:
>
>> I don't see how changing Molecule_type to Molecule::Type relieves the
>> problem.
>
> Within the set of code that works with Molecules, you only have to call
> it Type. (I'm not in love with that name, either.) This avoids the
> long names (which neither you nor I seem to like very much), without the
> need for abbreviating the English words used in those names.
>
> using Molecule::Type;
>
Molecules. In fact, Molecule_Type is a class that deals with a paricular
type (or species) of Molecule. It handles the properties (Mass etc.) of
that type (or species). There is a separate class (Molecule) that deals
with actual instances of Molecule Types.
So if I were going to use a namespace called Molecule; it would be for the
code handling the actual instances. I would still need a class / namespace
/ module to handle the Molecule Types (chemical species), and I would need
a name for that class / namespace / module that was both descriptive and
not unduly long.
Chris Gordon-Smith
> Pete Becker wrote:
>
>> How would you refactor a name like time_since_epoch(), which takes up
>> 20% of my line every time I have to use it?
>
> Why does 20% concern you? Just how many names per line are you trying
> to achieve?
>
>> Oh, and I can't change it:
>> it's part of the interface to a library that I don't control.
>
> Boo hoo. Nobody ever taught you about inline wrapper functions? If the
> meaning is already clear from context, use a simple name like "time".
But presumably time_since_epoch() is a particular kind of time function (as
opposed, for example to time_of_arrival()). If we use a name like 'time',
then it is short, but does not adequately describe what it is. Perhaps it
will sometimes be evident from the context, but this will not always be the
case. Even where the meaning can be inferred from context, this seems to
make the job of the reader harder than if a more descriptive name were
used.
Jeff Schwab
Yes, that's why I said "if the meaning is already clear from context."
Frankly, a name like time_since_epoch doesn't really bother me. Suppose
the name were a little longer, though, e.g. time_since_platform_epoch
(since the epoch is not the same on all platforms), or
time_since_posix_epoch, or seconds_since_posix_epoch_as<long long>. At
some point, it becomes unreasonable to pack all relevant information
into the name, and context-specific abbreviations become necessary. In
well-factored code, I think the abbreviations tend to be obvious; in
poorly factored code, people just resort to using arbitrary prefixes of
the individual words, or omitting the vowels, etc. You end up with
monstrosities like sec_since_ep_ll. Those get on my nerves.
Chris Gordon-Smith
> Chris Gordon-Smith wrote:
>>
>> But presumably time_since_epoch() is a particular kind of time function (as
>> opposed, for example to time_of_arrival()). If we use a name like 'time',
>> then it is short, but does not adequately describe what it is. Perhaps it
>> will sometimes be evident from the context, but this will not always be the
>> case. Even where the meaning can be inferred from context, this seems to
>> make the job of the reader harder than if a more descriptive name were
>> used.
>
> Yes, that's why I said "if the meaning is already clear from context."
> Frankly, a name like time_since_epoch doesn't really bother me. Suppose
> the name were a little longer, though, e.g. time_since_platform_epoch
> (since the epoch is not the same on all platforms), or
> time_since_posix_epoch, or seconds_since_posix_epoch_as<long long>. At
> some point, it becomes unreasonable to pack all relevant information
> into the name, and context-specific abbreviations become necessary. In
> well-factored code, I think the abbreviations tend to be obvious; in
> poorly factored code, people just resort to using arbitrary prefixes of
> the individual words, or omitting the vowels, etc. You end up with
> monstrosities like sec_since_ep_ll. Those get on my nerves.
OK. I think we are all agreed that names like sec_since_ep_ll are an
abomination. But how to avoid them without getting to excessively long
names? Are we saying that the way to do it is modularisation? By enclosing
a name within the scope of a namespace or class we enclose it within a
context, thereby limiting the number of words needed for the name to
adequately describe the thing (variable, function etc.) that it represents.
If this is the conclusion then its yet another benefit of modularisation.
Not only does it enable the complexity of large scale systems to be managed
and minimised, thereby helping to keep them maintainable. It also helps
readability of small scale items of the system by enclosing them within a
particular context and avoiding the need for very long names or crypic
abbreviations.
Jeff Schwab
Yes.
> If this is the conclusion then its yet another benefit of modularisation.
> Not only does it enable the complexity of large scale systems to be managed
> and minimised, thereby helping to keep them maintainable. It also helps
> readability of small scale items of the system by enclosing them within a
> particular context and avoiding the need for very long names or crypic
> abbreviations.
Yes again. To make it work, though, requires a lot of careful factoring
and refactoring of the code. I believe that factoring is nearly always
worthwhile, but clearly, not everyone agrees.