Initialize Pointer Through Function

Question

I was browsing my teacher's code when I stumbled across this:

Order* order1 = NULL;

then

order1 = order(customer1, product2);

which calls

Order* order(Customer* customer, Product* product)
{
    return new Order(customer, product);
}

This looks like silly code. I'm not sure why, but the teacher initialized all pointers to NULL instead of declaring them right away(looking at the code it's entirely possible, but he chose not to).

My question is: is this good or acceptable code? Does the function call have any benefits over calling a constructor explicitely? And how does new work in this case? Can I imagine the code now as kind of like:

order1 = new Order(customer, product);

Answer 1

If that's really all the important code, I see no benefit to the function or the initial NULL value. new works the way it always works. It constructs an Order and returns a pointer to it, which is in turn returned by order.

Answer 2

For the given code, there is no difference. Its better to initialize order object with a newly created object at single place. It will avoid using order variable initialized with NULL value

Answer 3

The code give could be important if the assignment to NULL happens in one function, e.g. the constructor, and the assignment that calls new happens in another function. Here's three reasons;

• If the customer and product parameters might not be available when the order = NULL was called.
• The NULL value could be significant in the interim to let other functions know that the order hasn't yet been created.
• If the Order class took a lot of resources, deferring its initialising could be beneficial.

Best to ask your teacher why they did it this way. Sometimes the obvious conclusions aren't the right ones.

Answer 4

Init to NULL

[edit] since there's a valid discussion, I've changed the order of the options a bit to emphasize the recommended option.

Variables should be declared as local and as late as possible, and initialized immediately. Thus, the most common pattern is:

Order * order1 = order(...);

just before order1 is required.
If there is any reason to separate the declaration of order1 from the instantiation, like this:

Order * order1;  // Oh no! not initialized!
// ... some code
order1 = order(...);

order1 should be initialized to NULL, to prevent common bugs that occur with uninitialized variables, easily introduced when // some code changes.

Factory method
Again, there's some more change resilence here: the requirements for instantiating an Order may change. There are two scenarios I can think of right off top of my head:

(1) Validation that can't be done by Order's constructor. Order may come from a 3rd party library and can't be changed, or instantiation needs to add validation that isn't within the scope of Order:

Order* order(Customer* customer, Product* product)             
{      
    // Order can't validate these, since it doesn't "know" the database       
    database.ValidateCustomer(customer); // throws on error
    database.ValidateProduct(product); // throws on error

    return new Order(customer, product);             
}   

(2) You may need an order that behaves differently.

class DemoOrder : public Order  { ... }

Order* order(Customer* customer, Product* product)             
{             
    if (demoMode)
      return new DemoOrder(customer, product); // doesn't write to web service
    else
      return new Order(customer, product);             
}   

However, I wouldn't make this a general pattern blindly.

Answer 5

It seems to me that your teacher is an old C programmer who hasn't quite shaken off some of his old habits. In the old times, you had to declare all variables at the beginning of a function, so it's not unusual to see some old timers still doing so.

Answer 6

If the things really are as you describe them then your teacher's code illustrates some rather bad programming practices.

Your question is tagged C++. In C++ the proper technique would be to move the variable declaration to the point where it can be meaningfuly initialized. I.e. it should've looked as follows

Order* order1 = order(customer1, product2);

The same would appliy to C99 code, since C99 allows declaring variable in the middle of the block.

In C code the declaration must be placed at the beginning of the block, which might lead to situations where you can't meaningfully initialize a variable at the point of declaration. Some people believe that in this case you have to initialize the variable with something, anything just to keep it initialized as opposed to leaving it uninitialized. I can respect this as a matter of personal preference, buy personally consider it counterproductive. It interferes with compiler optimizations and tends to hide bugs by sweeping them under the carpet (instead of encouraging proper fixes).

I would say that your teacher's C++ code suffers from some bad habits brought over from C.