Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I am talking about normal PC applications, memory should be sufficient.

They declare global or member variables.

In each function/method, they use the same global/member variables.

At the beginning of each method, the variables are re-initialized so the old value is never used.

Why do they keep the variables but no chance to reuse the old value?

class A
{
    List<Car> carList;
    public List<Car> M()
    {
        // always re-init variable
        this.carList = new List<Car>();
        // ...
        // fetch from DB
        return this.carList;
    }

    public List<Car> N(string id)
    {
        // always re-init variable
        this.carList = new List<Car>();
        // ...
        // fetch from DB
        return this.carList;
    }
}
share|improve this question
8  
This might be a misunderstanding about how member variables work vs. how local variables work. It seems like a local variable would be a better choice to me. Why not ask the programmer(s) who are doing this? –  Greg Hewgill Feb 20 '13 at 3:49
    
Consider the very likely possibility that it's an honest mistake. –  user16764 Feb 20 '13 at 4:04
1  
Shouldnt you be asking the programmers who did this? How are we supposed to know their motivation? –  GrandmasterB Feb 20 '13 at 5:04
    
"some programmers" - do you have evidence that this nonsense happens regularly for many classes written by many different programmers? –  Doc Brown Feb 20 '13 at 7:22
    
There is a big difference between instance fields and global variables/static fields. With instance fields it's just bad style, with global variables it's broken. –  CodesInChaos Feb 20 '13 at 10:07

5 Answers 5

It's impossible to know the motivations behind this code given a single point in time. It could be the result of wear-and-tear on the class.

Imagine, for instance, that once the car list really did function as a member variable, with some methods modifying it and some retrieving it. Then, some bugs were discovered that were caused by different methods modifying the value at the wrong time - perhaps the introduction of multithreaded execution. Now the programmer went and changed all car-list-generating methods to return the list, rather than store it in the member variable. But people often change the minimum amount required to get things working, especially if rushed and working on a critical bug, and rather than creating a local varible, he just left the existing reference to the member variable. Voila!

share|improve this answer
    
And before and sticklers comment - yes, the current code is vulnerable to multithreaded issues too, but requires a narrower race condition. –  Avner Shahar-Kashtan Feb 20 '13 at 5:53

They think that they are escaping declaring variable every time in each function. But this is not good programming habit to keep global variable if you every time initialize value. Life cycle of global variable may lead to bad memory management in this case

share|improve this answer

In your example, it might likely be bad to keep the old value. If you have constructed one list of cars in N and returned that list to caller A, and then caller B makes a request using M, and M cleared the same list and populated a different set of cars, wouldn't caller A perhaps be confused when suddenly the list mutated? It is more likely that Greg Hewgill's comment to your question is entirely valid, it never should have been member state in the first place.

What you see there is just a symptom of what often accompanies this, and that is a large class with members at the the wrong layers of abstraction, a class that is probably doing too much for too many people and can often sprawl to the point of being indecipherable. If, on the other hand, reinitialized member state is your only problem with the class, then maybe you're better off than those of us who have seen and worked with far worse.

share|improve this answer

It could be that you've got something like this:

class A
{
    List<Car> carList;
    public List<Car> M()
    {
        // always re-init variable
        this.carList = new List<Car>();
        // the following functions modify carList
        sortTheCarList();
        filterTheCarList();
        frobnicateTheCarList();
        return this.carList;
    }
}

It possible that it at least makes sense. Although I still think its bad style to do it this way.

share|improve this answer

Its not a good practice. Local variables should be used, if you are re-initialize the global variable every time in function. Global variables shoud be used if you want to retain the old values.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.