2

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;
    }
}
Improve this question
5
  • 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? Commented Feb 20, 2013 at 3:49
  • Consider the very likely possibility that it's an honest mistake. Commented Feb 20, 2013 at 4:04
  • 1
    Shouldnt you be asking the programmers who did this? How are we supposed to know their motivation? Commented Feb 20, 2013 at 5:04
  • "some programmers" - do you have evidence that this nonsense happens regularly for many classes written by many different programmers? Commented Feb 20, 2013 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. Commented Feb 20, 2013 at 10:07

4 Answers 4

Reset to default
4

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!

Improve this answer
1
  • 1
    And before and sticklers comment - yes, the current code is vulnerable to multithreaded issues too, but requires a narrower race condition. Commented Feb 20, 2013 at 5:53
3

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

Improve this answer
2

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.

Improve this answer
1
  • This shouldn't be an issue; according to the signatures the list is being returned by value, not by reference, so it'll be (shallow) copied for the caller when the method returns. The caller won't see the list mutate when the member variable is later modified, unless they're accessing the list via the member variable rather than the return value from the method (which would be crazy). Commented Feb 13, 2022 at 6:09
1

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.

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.