Is this good style of C++?

[Cheng Mo]

Recently I happens to read some sourcecode which has different style
from my previous experience.

In this style, all functions return a RETURN_TYPE_T which is defined as
typedef unsigned int RETURN_TYPE_T;
and the return value can only be enum value
enum
{
SUCCESS = 0,
FAILURE = 1,
ABORT = 2
};
If a function should return a object, the API is not like
Foo getFoo();
or
Foo* getFoo();
Instead, it is declared as
RETURN_TYPE_T getFoo(Foo* foo);
The caller has to new a Foo or get pointer to Foo object through other
way first, and then call this method with the pointer as parameter.

The function fill valuable info into the Foo object pointed by the
paramter. The result of invocation is judged by inspecting the returned
RETURN_TYPE_T value.

This style of coding seems strange to me, but I am told that it is a
good style, because it applys the rule "Who creates it, who releases
it". As a result, this style are supposed to reduce risk of memory leak.

I am still not quite convinced. Anybody with long time C++ exprirence
can give some comments?
Thanks & Regards

 

[Ioannis Vranos]

Recently I happens to read some sourcecode which has different style
from my previous experience.

In this style, all functions return a RETURN_TYPE_T which is defined as
typedef unsigned int RETURN_TYPE_T;
and the return value can only be enum value
enum
{
SUCCESS = 0,
FAILURE = 1,
ABORT = 2
};
If a function should return a object, the API is not like
Foo getFoo();
or
Foo* getFoo();
Instead, it is declared as
RETURN_TYPE_T getFoo(Foo* foo);
The caller has to new a Foo or get pointer to Foo object through other
way first, and then call this method with the pointer as parameter.

The function fill valuable info into the Foo object pointed by the
paramter. The result of invocation is judged by inspecting the returned
RETURN_TYPE_T value.

This style of coding seems strange to me, but I am told that it is a
good style, because it applys the rule "Who creates it, who releases
it". As a result, this style are supposed to reduce risk of memory leak.

I am still not quite convinced. Anybody with long time C++ exprirence
can give some comments?

If such a style is used in an application for a particular reason (that
is, it is not a style), then it is OK.


Otherwise as a *general style* of programming for C++ it looks primitive
and ancient.


If you want to handle errors you had better use exceptions, and not
return error code checks that this style implies.


If you want to create bullet proof code without memory leaks or *any
other resource* leaks, use "Resource Acquisition is Initialisation" (RAII):


http://groups.google.com/groups?q="[email protected]&rnum=1

 

[Peter Koch Larsen]

All in all its horrible.

Recently I happens to read some sourcecode which has different style from
my previous experience.

In this style, all functions return a RETURN_TYPE_T which is defined as
typedef unsigned int RETURN_TYPE_T;
and the return value can only be enum value
enum
{
SUCCESS = 0,
FAILURE = 1,
ABORT = 2
};

Do not use UPPERCASE_ONLY for values. These names should be reserved for
macroes only.

If a function should return a object, the API is not like
Foo getFoo();
or
Foo* getFoo();
Instead, it is declared as
RETURN_TYPE_T getFoo(Foo* foo);
The caller has to new a Foo or get pointer to Foo object through other way
first, and then call this method with the pointer as parameter.

Also horrible. The first signature should be preferred unless the call could
fail in a normal case in which case a smart pointer (see boost) would be
appropriate.

The function fill valuable info into the Foo object pointed by the
paramter. The result of invocation is judged by inspecting the returned
RETURN_TYPE_T value.

In general, using exceptions is far more reliable. There's no forgetting of
checking the return-type, and you would not have to obfuscate your code with
all that checking.

This style of coding seems strange to me, but I am told that it is a good
style, because it applys the rule "Who creates it, who releases it". As a
result, this style are supposed to reduce risk of memory leak.

This is good advice, but not so relevant for C++. Use RAII instead.

I am still not quite convinced. Anybody with long time C++ exprirence can
give some comments?
Thanks & Regards

Kind regards
Peter

 

[Ioannis Vranos]

Do not use UPPERCASE_ONLY for values. These names should be reserved for
macroes only.

Actually one convention is to name all constants with uppercase, be them
macros, const objects, or members of an enumeration.

 

[Kuba Ober]

If a function should return a object, the API is not like

If you want to handle errors you had better use exceptions, and not
return error code checks that this style implies.

Unless errors are frequent and expected, in which case exceptions are not
such a good choice. Throwing an exception typically has big overheads, so
unless given error is *exceptional*, it should be handled w/o throwing an
exception. Network code comes to mind, if transmission errors, timeouts
etc. are frequent.

Cheers, Kuba Ober

 

[Ioannis Vranos]

Unless errors are frequent and expected, in which case exceptions are not
such a good choice. Throwing an exception typically has big overheads, so
unless given error is *exceptional*, it should be handled w/o throwing an
exception. Network code comes to mind, if transmission errors, timeouts
etc. are frequent.

Well, my experience is in .NET, where every error is signified as an
exception and not as a return value, and so far I had no problems with
it. Also all the C++ standard library with the exception of the C
subset, uses exceptions to signify run-time errors.

 

[jeffc]

Unless errors are frequent and expected, in which case exceptions are not
such a good choice.

Unexpected behavior is pretty much the definition of an exception.

 

[Andre Kostur]

Well, my experience is in .NET, where every error is signified as an
exception and not as a return value, and so far I had no problems with
it. Also all the C++ standard library with the exception of the C
subset, uses exceptions to signify run-time errors.

I don't agree with that assessment. End-of-file on a stream, for example,
is _not_ an exception, neither is dynamic_casting a pointer to an unrelated
type. Offhand I can only think of a couple of places where an exception is
thrown from the Standard library, and that's because either it really is an
exceptional case (like running out of memory), or there is no other way to
indicate an error (like dynamic_casting a reference to an unrelated type,
or using .at() on a vector past the end of the vector).

 

[Ioannis Vranos]

I don't agree with that assessment. End-of-file on a stream, for example,
is _not_ an exception, neither is dynamic_casting a pointer to an unrelated
type.

These are not errors. End-of-file is just the end of a file and
dynamic_cast is actually a run-time test.

 

[Andre Kostur]

These are not errors. End-of-file is just the end of a file and
dynamic_cast is actually a run-time test.

Then I don't understand exactly what you would call an "error" then. IMHO:
If I try to read some data from a stream, and I don't get the data I was
expecting, then I'd call that an error. If I try to dynamic_cast a pointer
to an unrelated type, the runtime will return me a NULL to signify that
error. If I try to dynamic_cast a reference to an unrelated type, the
runtime will throw an exception to signify that error.

 

[Ioannis Vranos]

Then I don't understand exactly what you would call an "error" then.

In general, an unexpected situation.

IMHO:
If I try to read some data from a stream, and I don't get the data I was
expecting, then I'd call that an error.

Well, when you read the data from a stream, checking if its end has been
reached is a rational thing to do.

If I try to dynamic_cast a pointer
to an unrelated type, the runtime will return me a NULL

Actually a 0.

to signify that
error. If I try to dynamic_cast a reference to an unrelated type, the
runtime will throw an exception to signify that error.

Dynamic cast is aimed for "Run-time Type Identification" (RTTI), so this
is not an error. Consider this:



void someFunc(Base *p)
{
Derived1 *dp1= dynamic_cast<Derived1 *>(p);

if(dp1!= 0)
{
// ...

return;
}

Derived2 *dp2= dynamic_cast<Derived2 *>(p);

if(dp2!= 0)
{
// ...

return;
}

// ...
}

 

[E. Robert Tisdale]

In general, an unexpected situation.

The problem with using the term "error"
is that it requires a [subjective] value judgment.

C++ programmers prefer the more neutral term *exception*
to reference *rare* and *unpredictable* events
that are *expected* but cannot be *prevented*.

Programming errors (bugs), on the other hand,
are unexpected until they are discovered
at which point they are completely predicable
and can be prevented by fixing the bug.

Exceptions should be handled at the point
where they are detected if possible.
If the exception cannot be handled where it is detected
all of the information needed to handle the exception
in the calling program should be encapsulated in an exception object
and the exception should be returned or thrown.

Functions like

Foo getFoo(void);

which return a value may be used in expressions
so they must throw an exception
if it cannot be completely within getFoo(void)
and that exception *must* be caught and handled
by the calling program.

 

[Morgan Cheng]

This is good advice, but not so relevant for C++. Use RAII instead.

I read some doc got from google. It seems that RAII just a axiom that
requires that acquired resource should be released. However, this is not
easy to follow in C++ because if object is new-d in one part of code
and another part release it as design. It is quite possible that new-d
object is not released for human negligence.

"Who creates int, who releases it" emphasize that the part of code "new"
the object should take the responsibility to "delete" the object
explictly. With this discipline, it is not easy to cause memory leak.
IMO, this rule is more strict than RAII.

 

[Ioannis Vranos]

I read some doc got from google. It seems that RAII just a axiom that
requires that acquired resource should be released. However, this is not
easy to follow in C++ because if object is new-d in one part of code
and another part release it as design. It is quite possible that new-d
object is not released for human negligence.

"Who creates int, who releases it" emphasize that the part of code "new"
the object should take the responsibility to "delete" the object
explictly. With this discipline, it is not easy to cause memory leak.
IMO, this rule is more strict than RAII.

Check this:


http://groups.google.com/groups?q="[email protected]&rnum=1



When you manage your memory manually, there can always be a memory leak,
when an exception is thrown for example.


So you had better use RAII (= host objects in the stack) for memory and
other resources.


What does this mean? When you need to create a particular object in the
free store, you may use a smart pointer like auto_ptr, or create your
own host class for a particular resource.


For many objects use a standard library container like vector, list, etc.


So consider these examples:


// 10 Buttons created in the free store.
// Buttons are destroyed at the end of the scope, or when an exception
// is thrown.
vector<Buttons> options(10);


// p will automatically delete the pointed object
// at the end of its scope, or when an exception is thrown.
auto_ptr<Form> p(new Form);


// Use p as a Form *
p->Show();



// There are other resources apart from memory that can leak
class IPConnection
{
// ...

public:
IPConnection()
{
// Opens the connection ...
// Perhaps also new something;
}

~IPConnection()
{
// Closes the connection...
// delete something;
}
};


// Destructor is called at the end of the scope and when an
// exception is thrown, cleaning the resources.
IPConnection ip1;




All the above are RAII: Destructor is called at the end of the scope and
when an exception is thrown, cleaning the resources.

 

[Karl Heinz Buchegger]

I read some doc got from google. It seems that RAII just a axiom that
requires that acquired resource should be released. However, this is not
easy to follow in C++ because if object is new-d in one part of code
and another part release it as design. It is quite possible that new-d
object is not released for human negligence.

RAII as it is used in C++ is actually a simple thing to do in C++.
All you need to do, is to encapsulate the resource into a class of its
own. The constructor manages the allocation, the destructor releases
the resource. (+copy constructor, + assignment operator).
Users of that resource never deal with the dynamic allocation aspect. All
they ever use is an object of that class, and the object does all the
dynamic work.

Just think of std::string in contrast to plain-vanilla C-style strings.
std::string manages the C-style string under the hood. Users of std::string
never have to deal with that.

"Who creates int, who releases it" emphasize that the part of code "new"
the object should take the responsibility to "delete" the object
explictly. With this discipline, it is not easy to cause memory leak.
IMO, this rule is more strict than RAII.

RAII (as it is understood in C++) is exactly that principle encapsulated
in a class, with constructors and destructors doing all the work.

 

[Morgan Cheng]

Check this:


http://groups.google.com/groups?q="[email protected]&rnum=1



When you manage your memory manually, there can always be a memory leak,
when an exception is thrown for example.

If I implements RAII strictly, do you mean that memory leak can be
avoided totally? even exception is possibly to be thrown?

So you had better use RAII (= host objects in the stack) for memory and
other resources.


What does this mean? When you need to create a particular object in the
free store, you may use a smart pointer like auto_ptr, or create your
own host class for a particular resource.


For many objects use a standard library container like vector, list, etc.


So consider these examples:


// 10 Buttons created in the free store.
// Buttons are destroyed at the end of the scope, or when an exception
// is thrown.
vector<Buttons> options(10);


// p will automatically delete the pointed object
// at the end of its scope, or when an exception is thrown.
auto_ptr<Form> p(new Form);


// Use p as a Form *
p->Show();



// There are other resources apart from memory that can leak
class IPConnection
{
// ...

public:
IPConnection()
{
// Opens the connection ...
// Perhaps also new something;
}

~IPConnection()
{
// Closes the connection...
// delete something;
}
};


// Destructor is called at the end of the scope and when an
// exception is thrown, cleaning the resources.
IPConnection ip1;




All the above are RAII: Destructor is called at the end of the scope and
when an exception is thrown, cleaning the resources.

So, RAII is based on the fact that: At the end of a scope, through
normal running or exception thrown, objects in the scope will be
destructed. If resource is allocated explictly, objects pointed by
pointer will not be released by releasing the pointer. So, we wrap
dynamic resource allocation with a local object.
Am my understanding right?

 

[Ioannis Vranos]

If I implements RAII strictly, do you mean that memory leak can be
avoided totally? even exception is possibly to be thrown?

Yes, along with other resource types leaks (IP connections, file
operations etc).


All C++ standard library types support RAII (except of the C subset
types of course).

So, RAII is based on the fact that: At the end of a scope, through
normal running or exception thrown, objects in the scope will be
destructed.

Exactly.



If resource is allocated explictly, objects pointed by
pointer will not be released by releasing the pointer. So, we wrap
dynamic resource allocation with a local object.
Am my understanding right?

Yes.

 

[Ioannis Vranos]

Yes, along with other resource types leaks (IP connections, file
operations etc).


All C++ standard library types support RAII (except of the C subset
types of course).






Yes.

I want to mention here that using RAII in your applications is fairly easy.


The most obvious way is to use a vector for your objects (or some other
standard library container if it is more suitable), instead of an array
in the free store.


And you have no space/time cost doing this.

 

[Peter Koch Larsen]

Morgan Cheng wrote:

[snip]
If I implements RAII strictly, do you mean that memory leak can be avoided
totally? even exception is possibly to be thrown? Yes.[snip]
So, RAII is based on the fact that: At the end of a scope, through normal
running or exception thrown, objects in the scope will be destructed. If
resource is allocated explictly, objects pointed by pointer will not be
released by releasing the pointer. So, we wrap dynamic resource allocation
with a local object.
Am my understanding right?

Yes. Just never use raw pointers as holders of a ressource: encapsulate them
in a smart pointer (e.g. boost) and your (ressource) problems should
disappear.

Kind regards
Peter

 

[Dave O'Hearn]

Yes. Just never use raw pointers as holders of a ressource:
encapsulate them in a smart pointer (e.g. boost) and your
(ressource) problems should disappear.

Almost! A reference-counted smart pointer, like Boost shared_ptr, does
not solve the object ownership problem. It just changes its form from
manual new/delete to something like a reference-counted GC. This
eliminates dangling pointers, but it does not eliminate all memory
leaks. If you store a smart pointer away somewhere and forget about it,
you will leak memory.

I am mostly aware of this from Java, where I have seen a lot of code
that leaked memory in this manner. In some ways, dangling pointers are
easier to deal with than GC or reference counting, because the app will
(usually...) crash on the use of the dangling pointer. With "safe"
things like shared_ptr, the app will keep running, with no warning that
some object you thought you released the last reference to is still
referenced.