* Allen:
Yesterday I discussed with my boss about method naming.
In the RPC argument pack / unpack classes, packing & unpacking remote
procedure
calling arguments into / from a byte buffer. I named the methods as
following.
class PackRPCPara
{
public:
INT8 PackInt8(const INT8 value);
INT32 PackInt16(const INT16 value);
INT32 PackInt32(const INT32 value);
INT32 PackInt64(const INT64 value);
INT32 PackBoolean(const BOOL32 value);
INT32 PackBytes(const INT8 * value, const INT32 length);
INT32 PackString(const CHAR * value, const INT32 length);
};
He said, PackInt8, PackInt16, ... PackInt64 should use overrided method
name, i.e. Pack.
It is easy to use and low error prompting.
When you write "overrided" it seems you mean "overloaded".
Generally it's not a good idea to embed implementation issues in names,
so in that regard your Boss Is Right(TM).
However, say you have the following client code:
UINT8 const value = 255;
PackRPCPara packer;
packer.Pack( value );
Assuming INT8 is a signed type, this does not call the INT8 overload of
Pack. Most likely it calls the INT32 overload. So just naming all of
these functions "Pack" is probably not a good idea: it can introduce
silent errors, only detected at run time (if at all).
There is also another issue, that BOOL32 might be the same type as
INT32, in which case these functions can't both be named "Pack".
However, for the boolean you probably /want/ all kinds of boolean
argument to end up in the same function. So call that function e.g.
'addBool'.
By the way, if you have any choice in this matter, do reserve all
uppercase names for macros (see this group's FAQ as well as Bjarne
Stroustrup's FAQ): don't use them for types or typed constants.
I'd do
#define COMPILE_TIME_ASSERT( e ) int x[!!(e)] // Whatever.
namespace rpc
{
class PackedParameters
{
public:
template< typename IntType >
void add( IntType value ) { COMPILE_TIME_ASSERT( false ); }
template<void add<INT8>( INT8 value );
template<void add<INT16>( INT16 value );
// ... etc.
};
}
This way the erronous client code shown above won't compile, but must be
written as
parameters.add<INT8>( value );
or as
parameters.add( static_cast<INT8>( value ) );
Note, by the way, that the 'const' in the first five member functions
you've shown is entirely superflous and removed by the compiler.
I strongthly suggest it should be named as above. Because we are
developing a cross-platform
project. Overide Pack method uses implicit argment type calling
conducted by compiler. It is
not easy to read and difficult to maintain.
See above.
--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
