the c# return statement

M

Milo T.

This is a good one. If the idvalue is other than Square, Circle, Pentagon
or Sheep, then you have no return. To that I have to say that one of the
tennits of good coding, states that I cannot trust an object where I do
not control the memory in which it resides. The returned object is not in
the callers control and hence may be invalid. This may be fast but it is
very dangerous. There are far better patterns were reliability is required
and isn't that all the time.

Well, I wasn't trying to write solid and robust code, I was trying to spend
5 seconds coming up with examples where switch statements would be useful.

However, I appreciate your willingness in becoming a code reviewer for me,
though I would like to point out that the correct way to solve the above
problem is merely to add a default: return NULL; or default: throw
exception("Unknown object type in deserialization"); to the bottom of the
statement.

In future, I will remember to provide you with a fully compiled, zipped up
source tree with a make file instead of just a quick off the cuff example.

By the way, you missed the fact that I was missing a main() function as
well.
 
M

Milo T.

Better, because this kind of code usually consolidates the switch in the one
place. Note that this sort of construction doesn't come up that many times
in the same program, because you only have one of these for each family of
classes.


see std::map. The clean way to implement the above in c++ is to have a
std::map<int, Object*> and use a clone() method to get your prototype.
The translation unit that defines the subclass in question takes care of
inserting the instance into the map.

e.g.

template <class T> insert_into_map {
insert_into_map(std::string s) {
T* x = new T;
the_map().insert(s,x); // the_map is the global table
}
};

class Circle {

....

};
namespace {
insert_into_map <Circle> x("Circle);
};

Object* make_object(const std::string & id){
std::map<std::string,Object*>::iterator it = the_map().find(id);
if (it==the_map().end())
return NULL;
else return it->second->clone();
}

This also has the advantage that you can populate the map at runtime if you
dynamically load the code that defines circle (so you get runtime plugin
support)

It also has the disadvantage that you're using variable length strings as
your key for your map, you're using a tree for the lookup instead of a
hashtable or a trie (woe betide if you ever have to use this in a fast
fashion), and it adds to your initialization time. Not to mention the
potential memory cost of all of these Object's that you need to clone
later.

There's no point making code extensible unless you plan to extend it. Wrap
it in a function, and you can replace with an extensible version later.

Most cases, however, will survive perfectly well with the much faster
switch() statement.
 
T

The Ghost In The Machine

In comp.os.linux.advocacy, John Bailo
<[email protected]>
wrote
The c# *return* statement has been bothering me the past few months.

I don't like the fact that you can have different code paths in a method
and have multiple return statements. To me, it would be more orthogonal
if a method could only have one return statement.

Java has the exact same problem.

I'll admit to some curiosity but one way around this issue may be
at the code generation level of the compiler: briefly, if one codes

public int doSomething()
{
if(badThing())
return -1;

/* something */

if(anotherbadThing())
return -2;

int retv = /* whatever */;

/* more something */

return retv;
}

one might get:

* * *

_doSomething:

CALLS #0, badThing
TST.L R0
JZ $1
MOV.L #-1, R0
JMP _doSomething$Return

$1:
/* something */

CALLS #0, anotherbadThing
TST.L R0
JZ $2
MOV.L #-2, R0
JMP _doSomething$Return

$2:

/* whatever */

MOV.L (whatever),R1 ; we're assuming retv is cached in a reg here

/* more something */

MOV.L R1, R0

_doSomething$Return:

RET

* * *

where only one RET is in the routine -- and the compiler is responsible
for ensuring that each other return statement has R0 loaded properly
and jumping thereto.

(The assembly language is a corruption of VAX assembly, which I happen
to like. MOV a, b stores a into b. '#' indicates immediate.
R0 and R1 are registers. TST.L tests a value and sets condition
flags which JZ tests. JMP, erm, jumps. $n is a local label, a
useful concept in some assemblers. CALLS calls a routine, using
parameters on the stack; VAX also supports CALLG, which uses
a preformatted argument list -- useful for very old FORTRAN and
COBOL dialects that did not support recursion. RET of course returns.)

Of course I for one feel this is mostly a philosophical dispute.
If the issue is code clarity, one can work around it in various fashions.
 
J

John Bailo

The said:
In comp.os.linux.advocacy, John Bailo
<[email protected]>
wrote


Java has the exact same problem.

I'll admit to some curiosity but one way around this issue may be
at the code generation level of the compiler: briefly, if one codes

public int doSomething()
{
if(badThing())
return -1;

/* something */

if(anotherbadThing())
return -2;

int retv = /* whatever */;

/* more something */

return retv;
}

one might get:

* * *

_doSomething:

CALLS #0, badThing
TST.L R0
JZ $1
MOV.L #-1, R0
JMP _doSomething$Return

$1:
/* something */

CALLS #0, anotherbadThing
TST.L R0
JZ $2
MOV.L #-2, R0
JMP _doSomething$Return

$2:

/* whatever */

MOV.L (whatever),R1 ; we're assuming retv is cached in a reg here

/* more something */

MOV.L R1, R0

_doSomething$Return:

RET

* * *

where only one RET is in the routine -- and the compiler is responsible
for ensuring that each other return statement has R0 loaded properly
and jumping thereto.

(The assembly language is a corruption of VAX assembly, which I happen
to like. MOV a, b stores a into b. '#' indicates immediate.
R0 and R1 are registers. TST.L tests a value and sets condition
flags which JZ tests. JMP, erm, jumps. $n is a local label, a
useful concept in some assemblers. CALLS calls a routine, using
parameters on the stack; VAX also supports CALLG, which uses
a preformatted argument list -- useful for very old FORTRAN and
COBOL dialects that did not support recursion. RET of course returns.)

Of course I for one feel this is mostly a philosophical dispute.
If the issue is code clarity, one can work around it in various fashions.

what bothers me is that /return/ is both a *path* and a *value*

it sets a value to be passed to a method

*and*

it is a control statement -- offering break;

to me -- not good...
 
D

Donovan Rebbechi

It also has the disadvantage that you're using variable length strings as
your key for your map, you're using a tree for the lookup instead of a
hashtable or a trie (woe betide if you ever have to use this in a fast
fashion), and it adds to your initialization time. Not to mention the

My understanding is that dynamic allocation is pretty slow so it should
dwarf the other stuff, but I don't really know because I haven't benchmarked
it -- never needed this idiom for fast allocation of small objects (I try to
avoid heavily dynamic code for very small objects that need to be processed
quickly). Could also depend partly on how good the allocator is.

Cheers,
 
C

cody

I don't like the fact that you can have different code paths in a method
and have multiple return statements. To me, it would be more orthogonal
if a method could only have one return statement.

Java has the exact same problem.

I'll admit to some curiosity but one way around this issue may be
at the code generation level of the compiler: briefly, if one codes
[..]
where only one RET is in the routine -- and the compiler is responsible
for ensuring that each other return statement has R0 loaded properly
and jumping thereto.

(The assembly language is a corruption of VAX assembly, which I happen
to like. MOV a, b stores a into b. '#' indicates immediate.
R0 and R1 are registers.

Since when does the JVM use registers? It is stackbased.
 
C

cody

I don't like the fact that you can have different code paths in a method
and have multiple return statements. To me, it would be more orthogonal
if a method could only have one return statement.

Java has the exact same problem.

I'll admit to some curiosity but one way around this issue may be
at the code generation level of the compiler: briefly, if one codes
[..]
where only one RET is in the routine -- and the compiler is responsible
for ensuring that each other return statement has R0 loaded properly
and jumping thereto.

(The assembly language is a corruption of VAX assembly, which I happen
to like. MOV a, b stores a into b. '#' indicates immediate.
R0 and R1 are registers.

Since when does the JVM use registers? It is stackbased.
 
C

cody

I don't like the fact that you can have different code paths in a method
and have multiple return statements. To me, it would be more orthogonal
if a method could only have one return statement.

Java has the exact same problem.

I'll admit to some curiosity but one way around this issue may be
at the code generation level of the compiler: briefly, if one codes
[..]
where only one RET is in the routine -- and the compiler is responsible
for ensuring that each other return statement has R0 loaded properly
and jumping thereto.

(The assembly language is a corruption of VAX assembly, which I happen
to like. MOV a, b stores a into b. '#' indicates immediate.
R0 and R1 are registers.

Since when does the JVM use registers? It is stackbased.
 
D

Daniel O'Connell [C# MVP]

cody said:
I don't like the fact that you can have different code paths in a
method
and have multiple return statements. To me, it would be more orthogonal
if a method could only have one return statement.

Java has the exact same problem.

I'll admit to some curiosity but one way around this issue may be
at the code generation level of the compiler: briefly, if one codes
[..]
where only one RET is in the routine -- and the compiler is responsible
for ensuring that each other return statement has R0 loaded properly
and jumping thereto.

(The assembly language is a corruption of VAX assembly, which I happen
to like. MOV a, b stores a into b. '#' indicates immediate.
R0 and R1 are registers.

Since when does the JVM use registers? It is stackbased.

JIT'ers are pretty much free to use whatever registers the cpu exposes as it
wishes, only the virtual machine itself is stack based. The above assembly
is a pretty bad choice to illustrate the point, considering its considerably
different than the architecture we are used to and it doesn't illustrate the
bytecode emitted by C#\Java\what have you. However the same thing could be
showing in IL or x86 assembly. You rarely see more than one ret instruction
in a routine, although I'm pretty sure thats nothing more than an assembler
restriction in mosts cases. I believe some assemblers I've used use ret to
close a proc instead of a specific end proc keyword or whatnot. There is no
restriction in any cpu I know of that states only one ret per proc(infact no
cpu's I know well enough to say much about know what a proc is). Virtual
machines like the CLR and the JVM may require only one ret per routine, but
that would likely be determined by the verifier, not the execution unit
itself.

I would think the most common reason for ret being at the end is that much
of the time, a return requires restoring the register state or any other
state to what it was before the routine call, excluding of course the
changes the routine did. It is considerably more efficent to place that code
in the routine once instead of writing it for every return point. Hence a
jump is logical.

This isn't the case in a higher level language, where the compiler worries
about maintaining state.
--
cody

[Freeware, Games and Humor]
www.deutronium.de.vu || www.deutronium.tk
 
I

Ian Hilliard

Well, I wasn't trying to write solid and robust code, I was trying to
spend 5 seconds coming up with examples where switch statements would be
useful.

However, I appreciate your willingness in becoming a code reviewer for
me, though I would like to point out that the correct way to solve the
above problem is merely to add a default: return NULL; or default: throw
exception("Unknown object type in deserialization"); to the bottom of
the statement.

In future, I will remember to provide you with a fully compiled, zipped
up source tree with a make file instead of just a quick off the cuff
example.

By the way, you missed the fact that I was missing a main() function as
well.

I didn't know you were looking for a code review for this code fragment.
Returning NULL could cause undefined results in the calling class. An
exception should only be used in exceptional cases. The best solution is
to have a null class, which inherits from Object, which is to be returned
in the error case. The null class can be tested on return to ensure that
it is NULL, without causing anything to break should it be used without
first being tested.

i.e.
//Allocation of static
NullObject MyClass::nullObject;

Object* MyClass::ConstructMeAnObject(unsigned short idvalue) {
switch (idvalue)
{
case Square:
return new Square();
case Circle:
return new Circle();
case Pentagon:
return new Pentagon();
case Sheep:
return new Sheep();
default:
return &nullObject:

}
}

I won't go into the fact that returning a reference is considered more
reliable than returning a pointer. Also, it is often better to use
precreated objects from a pool rather than creating new ones.

Ian
 
M

Milo T.

I didn't know you were looking for a code review for this code fragment.
Returning NULL could cause undefined results in the calling class.

Which is why as part of the contract of the method, you specify that NULL
indicates an invalid type was requested.
An
exception should only be used in exceptional cases.

Which, if unknown types are found in the serialized stream, is indeed an
exceptional case.
The best solution is
to have a null class, which inherits from Object, which is to be returned
in the error case.

Ahhh... fail silently... ok. That sounds like a wonderful idea.
The null class can be tested on return to ensure that
it is NULL, without causing anything to break should it be used without
first being tested.

You're doubling the chances of a programming error - because not only now
can the user keep using the object without knowing that it's invalid, but
you're asking them to perform an entirely separate test for whether or not
the object is valid before using it. If you pass them a NULL pointer,
however, they cannot use it without testing it for NULL - if they use it
without testing, they get very visible feedback of the flaw.
i.e.
//Allocation of static
NullObject MyClass::nullObject;

Object* MyClass::ConstructMeAnObject(unsigned short idvalue) {
switch (idvalue)
{
case Square:
return new Square();
case Circle:
return new Circle();
case Pentagon:
return new Pentagon();
case Sheep:
return new Sheep();
default:
return &nullObject:

}
}

I won't go into the fact that returning a reference is considered more
reliable than returning a pointer.

Really? Why? If your base classes have virtual destructors (which all base
classes should have), then there's no difference. Also note that we're
creating new instances of objects here - a case for which passing
references is not designed.
Also, it is often better to use
precreated objects from a pool rather than creating new ones.

I welcome you to generate precreated paragraphs of text from a pool for use
in a wordprocessor application. There's a world of difference between using
preallocated memory chunks from a pool using placement-new and using
pre-constructed objects.

As I said, next time I'll provide you with a full application + makefile,
plus an "I'm A Pedant On A Tangent Rampage" pin badge.
 
G

Guest

On second thought, please ignore me. I was having a mental breakdown back there trying to figure out some stuff under /etc/fstab

Thanks all.
 
I

Ian Hilliard

Which is why as part of the contract of the method, you specify that NULL
indicates an invalid type was requested.


Which, if unknown types are found in the serialized stream, is indeed an
exceptional case.


Ahhh... fail silently... ok. That sounds like a wonderful idea.


You're doubling the chances of a programming error - because not only now
can the user keep using the object without knowing that it's invalid, but
you're asking them to perform an entirely separate test for whether or not
the object is valid before using it. If you pass them a NULL pointer,
however, they cannot use it without testing it for NULL - if they use it
without testing, they get very visible feedback of the flaw.


Really? Why? If your base classes have virtual destructors (which all base
classes should have), then there's no difference. Also note that we're
creating new instances of objects here - a case for which passing
references is not designed.


I welcome you to generate precreated paragraphs of text from a pool for use
in a wordprocessor application. There's a world of difference between using
preallocated memory chunks from a pool using placement-new and using
pre-constructed objects.

As I said, next time I'll provide you with a full application + makefile,
plus an "I'm A Pedant On A Tangent Rampage" pin badge.

Effectively you are using the state pattern. By defining a NULL state, the
operations within that NULL state can indicate that it is an undefined
state and the system can react accordingly.

There are also many times when base classes cannot use virtual
destructors, this is particularly the case when it is a non-virtual base
class that needs to clean up its own private members on destruction.

References are safer as noone can come up with the stupid idea of
incrementing a reference, by accident. Even better is a const reference,
but that assumes that there is no state change within the returned class,
that will be caused by the operations upon it. Hence the available methods
can be made const.

Precreating the paragraphs may be of use, if the system can accept that
level of granularity. Of more use may be creation of word level objects to
keep to your analogy. By precreating the objects, time is saved that would
otherwise be used for creating the objects. If all the objects that will
be required are preconstructed then there is no chance that an object will
not be available. The is common practice on real time systems.

I am not a pedant, on the contrary, I spend my days getting projects
running and one of the most common things that stops a project for
successful completion is sloppy programming that leaves deeply embedded
bugs. The patterns required for clean development in C++ are well known
and well documented. I suggest you learn them.

Ian
 
M

Milo T.

Effectively you are using the state pattern. By defining a NULL state, the
operations within that NULL state can indicate that it is an undefined
state and the system can react accordingly.

No, I believe you'll find that I'm using the Factory pattern.
There are also many times when base classes cannot use virtual
destructors, this is particularly the case when it is a non-virtual base
class that needs to clean up its own private members on destruction.

Repeat after me: base classes should ALWAYS have virtual destructors.
References are safer as noone can come up with the stupid idea of
incrementing a reference, by accident. Even better is a const reference,
but that assumes that there is no state change within the returned class,
that will be caused by the operations upon it. Hence the available methods
can be made const.

How do you propose then, in the above code, that I return references?

How about a small code sample?
Precreating the paragraphs may be of use, if the system can accept that
level of granularity. Of more use may be creation of word level objects to
keep to your analogy. By precreating the objects, time is saved that would
otherwise be used for creating the objects. If all the objects that will
be required are preconstructed then there is no chance that an object will
not be available. The is common practice on real time systems.

Nope. Sorry. It's not common practice on real time systems. Pre-allocating
memory is common practice. Pre-creating the objects themselves is not.
I am not a pedant, on the contrary, I spend my days getting projects
running and one of the most common things that stops a project for
successful completion is sloppy programming that leaves deeply embedded
bugs. The patterns required for clean development in C++ are well known
and well documented. I suggest you learn them.

I suggest you learn them too, Ian. Because you seem to be mixing up the
Factory pattern with the State pattern. You're mixing up pre-allocating
memory (or using a small-block/fixed-block memory allocator with placement
new) with pre-instantiating objects. You don't even know that all classes
which are derived from SHOULD ALWAYS have a virtual destructor - and you
don't seem to know that not having that leaves deeply embedded bugs and
indicates very sloppy programming.

Which means that your learnin' is sloppy, boy.

May I recommend reading Design Patterns by the GOF, and all of Scott
Meyers' books? It would appear that you need a refresher at least.
 
I

Ian Hilliard

I suggest you learn them too, Ian. Because you seem to be mixing up the
Factory pattern with the State pattern. You're mixing up pre-allocating
memory (or using a small-block/fixed-block memory allocator with placement
new) with pre-instantiating objects. You don't even know that all classes
which are derived from SHOULD ALWAYS have a virtual destructor - and you
don't seem to know that not having that leaves deeply embedded bugs and
indicates very sloppy programming.

Which means that your learnin' is sloppy, boy.

May I recommend reading Design Patterns by the GOF, and all of Scott
Meyers' books? It would appear that you need a refresher at least.

You can only make the statement that base classes have virtual destructors
if they have no state themselves. Have a look at this class and tell me if
there is the need for a virtual destructor or not.

/**START[HeaderBlock]-----------------------------------------------------
*
* Filename: HSocket.cpp
*
* Last Modified date:
* Tuesday, 19 September 2000
* Author :
* Ian Hilliard
*
* Description:
* General purpose socket class for connection oriented and conectionless
* operation. This class is also intended for use with HSocketPool for
* managing multiple sockets within a single thread.
*
*END[HeaderBlock]---------------------------------------------------------
*/
#include "StdAfx.h"
//*START[ApplicationIncludes]---------------------------------------------
#include "HSocket.h"
//*END[ApplicationIncludes]-----------------------------------------------
//*START[StaticDefinitions]-----------------------------------------------
//*END[StaticDefinitions]-------------------------------------------------
/**-----------------------------------------------------------------------
* Operation: Default Constructor
* Create an instance of HSocket where the backlog is specified. If *
backlog is not specified, a default value of 8 is used.
*-------------------------------------------------------------------------
* Parameters:
* int backLog
* The number of connections that can be accepted without being serviced.
*-------------------------------------------------------------------------
* Returns:
* void
*-------------------------------------------------------------------------
*/
HSocket::HSocket( int backLog )
: m_backlog( backLog ),
m_initialised( false )
{
// Initialise Winsock to use version 2.2. if( WSAStartup( MAKEWORD( 2, 2
), & m_wsa ) != 0 ) {
m_exception.addEntry( (int) sockerr_wsaStartup, WSAGetLastError() );
throw m_exception;
}
}
//*END[Operation]---------------------------------------------------------


/**-----------------------------------------------------------------------
* Operation: ~HSocket
* Default Destructor
*-------------------------------------------------------------------------
*/
HSocket::~HSocket( )
{
closesocket( m_socket );
WSACleanup();
}
//*END[Operation]---------------------------------------------------------

If HSocket had a vitrual destructor, then it would be necessary for each
child to do it's own clean up. And as the saying goes; "Children are
not permitted to play with their parent's privates."

There are also other problems with virtual destructors. It looks like
you're another one of those programmers who read Meyers and now thinks he
can program. The important thing with programming is understanding what
it is that the compiler is doing with the code.

As far as preallocating goes, best practice demands that the objects be
preallocated and then initialized as required. I suggest that you read
"Advanced C++ Programming Styles and Idioms" by James O. Coplien.

As far as returning const references, that is very simple

const State & MyClass::getState( Event transitionEvent,
const State & previousState )
{
switch( transitionEvent )
{
case pouring:
m_milkState.setState( previousState );
return( m_milkState );

case curdling:
m_curdsState.setState( previousState );
return( m_curdsState );

case separating:
m_cheeseState.setState( previousState );
return( m_cheeseState ):

case churning:
m_butterState.setState( previousState );
return( m_butterState );

default:
return( m_unknownState );
}
}

Using this pattern, you get a new set of functionality based on the new
state. The new state is set to reflect the previous state. Each of these
state objects provides state functionality with the only possible changes
to their state being made by the state factory. This is a very efficient
variant on the state pattern.

It should be noted that the entry point of each state is the same virtual
method call. Hence, the calling class can simply use the returned reference
and make the same method call.

Ian
 
M

Milo T.

You can only make the statement that base classes have virtual destructors
if they have no state themselves. Have a look at this class and tell me if
there is the need for a virtual destructor or not.

If it's being used as a base class - that is, other classes inherit from it
- then it will need a virtual destructor, unless you're able to force users
of your class library to always destroy on the most-derived form of the
class.

/**START[HeaderBlock]-----------------------------------------------------
*
* Filename: HSocket.cpp
*
* Last Modified date:
* Tuesday, 19 September 2000
* Author :
* Ian Hilliard
*
* Description:
* General purpose socket class for connection oriented and conectionless
* operation. This class is also intended for use with HSocketPool for
* managing multiple sockets within a single thread.
*
*END[HeaderBlock]---------------------------------------------------------
*/
#include "StdAfx.h"
//*START[ApplicationIncludes]---------------------------------------------
#include "HSocket.h"
//*END[ApplicationIncludes]-----------------------------------------------
//*START[StaticDefinitions]-----------------------------------------------
//*END[StaticDefinitions]-------------------------------------------------
/**-----------------------------------------------------------------------
* Operation: Default Constructor
* Create an instance of HSocket where the backlog is specified. If *
backlog is not specified, a default value of 8 is used.
*-------------------------------------------------------------------------
* Parameters:
* int backLog
* The number of connections that can be accepted without being serviced.
*-------------------------------------------------------------------------
* Returns:
* void
*-------------------------------------------------------------------------
*/
HSocket::HSocket( int backLog )
: m_backlog( backLog ),
m_initialised( false )
{
// Initialise Winsock to use version 2.2. if( WSAStartup( MAKEWORD( 2, 2
), & m_wsa ) != 0 ) {
m_exception.addEntry( (int) sockerr_wsaStartup, WSAGetLastError() );
throw m_exception;
}
}
//*END[Operation]---------------------------------------------------------


/**-----------------------------------------------------------------------
* Operation: ~HSocket
* Default Destructor
*-------------------------------------------------------------------------
*/
HSocket::~HSocket( )
{
closesocket( m_socket );
WSACleanup();
}
//*END[Operation]---------------------------------------------------------

If HSocket had a vitrual destructor, then it would be necessary for each
child to do it's own clean up. And as the saying goes; "Children are
not permitted to play with their parent's privates."

I'm not entirely certain what you're claiming here. All destructors get
called in order, from most-derived child (the instance type) to root base
class. The children don't need to do their own clean up - other than for
their own resources.

Perhaps you'd like to explain your thinking on this one.

Oh, and by the way... you might want to use a singleton pattern on that
Winsock DLL initialize/cleanup pair. Either that, or break the
initialization of winsock out of the socket class entirely. Where it stands
now, you're making a whole slew of wasted calls.
There are also other problems with virtual destructors.

Only when serializing classes as structures directly to another storage
location.
It looks like
you're another one of those programmers who read Meyers and now thinks he
can program. The important thing with programming is understanding what
it is that the compiler is doing with the code.

Ian, you've already proven that you were asleep in class when they covered
Object Factories. Now you've proven that you were asleep when they covered
virtual functions and destructor behavior as well. Would you care to wake
up at any point in the near future?

You've got some nerve telling me that I'm "one of those programmers who
read Meyers and now thinks he can program"... given that so far you've had
several huge mistakes in your "learning".
As far as preallocating goes, best practice demands that the objects be
preallocated and then initialized as required. I suggest that you read
"Advanced C++ Programming Styles and Idioms" by James O. Coplien.

Bzzzzzzttt... you said - and I quote:

"By precreating the objects, time is saved that would
otherwise be used for creating the objects. If all the objects that will
be required are preconstructed then there is no chance that an object will
not be available. The is common practice on real time systems."

I'm the one who told you what the difference between preallocation and
precreation was. Stop changing your tune just because you've been caught
out and proven incompetent.
As far as returning const references, that is very simple

const State & MyClass::getState( Event transitionEvent,
const State & previousState )
{
switch( transitionEvent )
{
case pouring:
m_milkState.setState( previousState );
return( m_milkState );

case curdling:
m_curdsState.setState( previousState );
return( m_curdsState );

case separating:
m_cheeseState.setState( previousState );
return( m_cheeseState ):

case churning:
m_butterState.setState( previousState );
return( m_butterState );

default:
return( m_unknownState );
}
}

Yet another time where you misread Factory patterns for State patterns.
Please show the exact example I gave - that you said returning references
would be suitable for.

In case you've forgotten, this means that you must return a reference from
a function which looks like this:

Object* CreateSomething()
{
return new Object();
}

So go ahead, Ian. Rewrite that function to use a reference as you claimed
could be done. I'm waiting.
Using this pattern, you get a new set of functionality based on the new
state. The new state is set to reflect the previous state. Each of these
state objects provides state functionality with the only possible changes
to their state being made by the state factory. This is a very efficient
variant on the state pattern.

Bzzzzzzzzt. Sorry, but that's not a Factory pattern. That's a State
pattern. My example - that you were complaining about - was a FACTORY
pattern.
It should be noted that the entry point of each state is the same virtual
method call. Hence, the calling class can simply use the returned reference
and make the same method call.

I'm sure that's nice and all. However, it's entirely irrelevant.

With your proven attention to detail, it has to be said, your customers are
probably jumping up and down with glee... or at least jumping up and down
with something. Probably pitchforks.
 
I

Ian Hilliard

const State & MyClass::getState( Event transitionEvent,
const State & previousState )
{
switch( transitionEvent )
{
case pouring:
m_milkState.setState( previousState );
return( m_milkState );

case curdling:
m_curdsState.setState( previousState );
return( m_curdsState );

case separating:
m_cheeseState.setState( previousState );
return( m_cheeseState ):

case churning:
m_butterState.setState( previousState );
return( m_butterState );

default:
return( m_unknownState );
}
}
So go ahead, Ian. Rewrite that function to use a reference as you
claimed could be done. I'm waiting.

What I have shown you is a case where the objects are pre-existing, they
are initialized before use and a const reference is returned. I suggest
that you look more closely before you head off on your normal tangent.

I suggest that you go back to Gamma, you will see that the factory pattern
that you originally showed is part of a state system. If I remember
correctly, it was for creating the correct state objects for a game. I am
not confusing a state pattern with a factory. I recognise where that
paticular factory pattern is used. It has little sense being used else
where. On the other hand, perhaps you don't realize what you were doing
when using that pattern.

The comment that I made earlier if you care to go back is that it is
better to have preexisting objects, rather than creating them on the fly.
in the original example you showed, you are creating the objects on the
fly, using new. That takes a considerable amount of time to create new
objects, which could be fatal in a real-time system. If you had precreated
the object and simply returned a reference, there there would be a
considerable time saving, at run time. I realize that you could have
overwritten new to provide objects from a pool. I personally find that
method somewhat obviscated and as such tends to reduce maintainability of
the code.

I guess that you should just stick to Windows programming. At least then,
when you code flakes out, you can always blame the OS.

Ian
 
M

Milo T.

const State & MyClass::getState( Event transitionEvent,
const State & previousState )
{
switch( transitionEvent )
{
case pouring:
m_milkState.setState( previousState );
return( m_milkState );

case curdling:
m_curdsState.setState( previousState );
return( m_curdsState );

case separating:
m_cheeseState.setState( previousState );
return( m_cheeseState ):

case churning:
m_butterState.setState( previousState );
return( m_butterState );

default:
return( m_unknownState );
}
} [Ian SNIPPPPPPed here]
So go ahead, Ian. Rewrite that function to use a reference as you
claimed could be done. I'm waiting.

Actually, Ian, I didn't ask you to rewrite THAT function. Be more careful
with your quoting - someone might think you were trying to twist the
argument.
What I have shown you is a case where the objects are pre-existing, they
are initialized before use and a const reference is returned. I suggest
that you look more closely before you head off on your normal tangent.

Congratulations. But that has nothing to do with the design pattern at hand
- which is used in the case of deserialization of an object from storage.
I suggest that you go back to Gamma, you will see that the factory pattern
that you originally showed is part of a state system.

Nope. Sorry. If you read the book, you wil find that the Factory pattern is
part of the Creational Patterns section of the book.

The State pattern is part of the Behavioral Patterns section of the book.

You have read the book, haven't you?

Here's a refresher course for you:
http://www.codeproject.com/csharp/csdespat_1.asp

Read the Factory Pattern section.
If I remember
correctly, it was for creating the correct state objects for a game. I am
not confusing a state pattern with a factory. I recognise where that
paticular factory pattern is used. It has little sense being used else
where. On the other hand, perhaps you don't realize what you were doing
when using that pattern.

Please, therefore, explain to me what pattern you use when
serializing/deserializing objects from a store. Particularly the
deserialization case, oh Mighty Brainiac.
The comment that I made earlier if you care to go back is that it is
better to have preexisting objects, rather than creating them on the fly.
in the original example you showed, you are creating the objects on the
fly, using new. That takes a considerable amount of time to create new
objects, which could be fatal in a real-time system. If you had precreated
the object and simply returned a reference, there there would be a
considerable time saving, at run time.

Again you mix up preallocation of memory and pre-instantiation of objects.
I realize that you could have
overwritten new to provide objects from a pool. I personally find that
method somewhat obviscated and as such tends to reduce maintainability of
the code.

Pity you don't need to override new to provide that kind of functionality
then. Pity for your knowledge of C++, that is.
I guess that you should just stick to Windows programming. At least then,
when you code flakes out, you can always blame the OS.

I'm sorry Ian, I thought that was the excuse you used. It certainly seems
to be. After all, you're doing Windows programming (see your HSocket clas),
and getting most of your C++ knowledge wrong.

So far, you've had the following problems in your analyses:

IAN HILLARD'S C++ ARCHITECTURE, DESIGN AND PROGRAMMING SCORECARD
================================================================

1. No virtual destructors on base classes. (Class design)
2. Initialization/destruction of Winsock on a per-socket creation/teardown
level. (Perf)
3. Inability to recognize Factory pattern and that its use is separate from
State patterns. (Patterns)
4. Misunderstanding of how virtual destructors work, claiming that "each
child object would have to clean themselves up, and they're not supposed to
touch their parent code". (basic C++)
5. Mixing up pre-allocating memory from a pool with pre-instantiating
objects in a pool. (Perf)
6. Deciding to return fully-created objects from a factory in the case
where an invalid object type is specified, instead of returning a NULL
pointer, leading to more checks in code which consumes your class.
(robustness)
7. Claiming that asking a Factory to create an invalid type is not worthy
of throwing exceptions. (Class design).
8. Deciding to fail silently when asked to create invalid objects
(Design/Robustness)
9. Not understanding how placement-new works - clue: it doesn't involve
"overriding new". (Advanced C++)
10. Not having read Design Patterns more than once, or not having
understood what you were reading. (Basic English Comprehension/Amnesia)
 
I

Ian Hilliard

IAN HILLARD'S C++ ARCHITECTURE, DESIGN AND PROGRAMMING SCORECARD
================================================================

1. No virtual destructors on base classes. (Class design)
2. Initialization/destruction of Winsock on a per-socket creation/teardown
level. (Perf)
3. Inability to recognize Factory pattern and that its use is separate from
State patterns. (Patterns)
4. Misunderstanding of how virtual destructors work, claiming that "each
child object would have to clean themselves up, and they're not supposed to
touch their parent code". (basic C++)
5. Mixing up pre-allocating memory from a pool with pre-instantiating
objects in a pool. (Perf)
6. Deciding to return fully-created objects from a factory in the case
where an invalid object type is specified, instead of returning a NULL
pointer, leading to more checks in code which consumes your class.
(robustness)
7. Claiming that asking a Factory to create an invalid type is not worthy
of throwing exceptions. (Class design).
8. Deciding to fail silently when asked to create invalid objects
(Design/Robustness)
9. Not understanding how placement-new works - clue: it doesn't involve
"overriding new". (Advanced C++)
10. Not having read Design Patterns more than once, or not having
understood what you were reading. (Basic English Comprehension/Amnesia)

Clearly you still don't understand or you are trying to be obtuse.

1) I stated that it is necessary to understand what is happening. If the
parent has state which it needs to clean up, then the destructor cannot be
virtual as the destructor of the parent class will not be called if
operating on a child class. I guess that you only use parent classes with
no state or you let them leak.

2) If each class ensures that its requirements are met, there can never
be a case where the requirements are not met (robustness)

3) Once again you are being wantonly obtuse.

4) Now repeat after me "If a parent class has its own objects that it
needs to clean up, the destructor on the parent cannot be virtual."
Appearantly you don't understand that. I feel sorry for your employer.

5) Allocation of memory is not the only problem. It takes a considerable
amount of time to create complex objects. Hence, it is important to
precreate complex objects. It is also good practice to precreate all the
objects that are required so that they do not need to be created on the
fly. I guess that you're just another Windows programmer who thinks that
being frugal with resources is not important. You can always throw a
bigger processor at the problem.

6) If you return a NULL, it is necessary to test for NULL or expect the
code to crash. If there can never be a case where an invalid object can be
returned then in the case of failure, the returned object can provide the
failure behaviour. Once again, I guess that that is too complex a concept
for you. You would rather crack nuts with a sledge hammer.

7) Exceptions are very costly in resources and further more many operating
systems do not support them (Include Windows CE here). Even where
exceptions are available they should only be used in exceptional
circumstances.

8) Good real-times fail over a problem and then carry on. There is
generally no operator there to enjoy the BSOD which you initiated.

9) I understand fully how new operates, but every time you create an
object it costs time and resources. More over ever object on the heap is a
potential memory leak if things go wrong.

10) As I've pointed out the parameterized factory makes the great core for
a state machine. I suggest that you look at Gamma p114 and you see that
the pattern is used for creating the new state for some form of game, in
the Sample Code.

There are also other important things to be remembered with any code being
written.
a) Within the lifetime of any complex system there are likely to be a
number of changes in the environment in which it needs to work. It is
therefore important not to write code that can only be run on one platform.
b) Code will always be asked to do more than it was originally designed
for. It is hence important to not waste resources as they will probably be
needed by the live system.

It is nice to see that you have a alot of room for improvement. It is
never too late to start.

Ian
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top