CODEWITHSUNDEEP
c#
PositiveGuy
PositiveGuy

Reputation: 47763

Is this really saving allocation?

I've got the following in a static Utility class:

static int[] MonthDays = new int[] {1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31};
public static int[] GetListOfAllDaysForMonths()
{
    return MonthDays;
}

Is this efficient in that now by calling GetListOfAllDaysForMonths() in several places in code, that I am saving myself from having to allocate and create a new int[] each time this method is called?

Updated: Lets make it readonly

 static readonly int[]

Upvotes: 2

Views: 261

Answers (11)

Powerlord
Powerlord

Reputation: 88816

If this is truely to populate a dropdown, why not use DateTime's DaysInMonth static method to get the number of days given the year and month, then create an array containing those values (using Enumerable.Range or some such) to populate the dropdown?

DateTime.DaysInMonth(2009, 10); // returns 31
DateTime.DaysInMonth(2009, 2); // returns 28
DateTime.DaysInMonth(2012, 2); // returns 29
DateTime.DaysInMonth(2009, 9); // returns 30

Upvotes: 6

Matt Briggs
Matt Briggs

Reputation: 42238

Even if it makes a difference, it is so small as to make it completely irrelevant. Micro optimizations are pointless (except for the few times they aren't, the only way you will know what those are is if you run into bottlenecks). Don't let the allocation of a 12 element array bother you, instead focus on algorithmic efficacy, because that is where you will see a difference.

Upvotes: 2

Powerlord
Powerlord

Reputation: 88816

Have you considered using the built-in ASP.NET Calendar control rather than rolling your own date picker?

Upvotes: 1

Ray
Ray

Reputation: 21905

If this is used just to populate drop downs, then how about a method like this:

populateDaysList(ListItemsCollection items) {
  items.Add(new ListItem("1"));
  items.Add(new ListItem("2"));
  items.Add(new ListItem("3"));
  ...
  items.Add(new ListItem("31"));
}

No need for an array or an iterator (for or foreach) to read through it

Upvotes: 0

Michael Baldry
Michael Baldry

Reputation: 792

why optimise this? it's premature, the value you get out of doing this is probably so small you'll never notice it. If it ever did become a problem, then that is the time you optimise, doing so before hand is a waste of time, resource and can make code unnecessarily complicated and messy.

Upvotes: 1

Fredrik Mörk
Fredrik Mörk

Reputation: 158369

Yes, but you also open for some interesting side effects. Consider this:

int[] a = GetListOfAllDaysForMonths();
a[3] = 200;
int[] b = GetListOfAllDaysForMonths();
Console.WriteLine(b[3]);  // prints 200

Update
Given the intended use (default numbers for day-of-month in dropdown lists), you will need many (and I mean many, as in hundreds of thousands or millions) such lists for this kind of optimization to make a difference. I would not spend time trying to optimize this from memory allocation perspective; it simply will not be worth it.

For now, I would recommend you to just have a method return a new int[] (exposed as an IEnumerable<int>) with the numbers for each call. Then, at a later date, if you happen to identify this to pose a problem with memory or performance, then you can optimize that method.

Upvotes: 10

Andrew Hare
Andrew Hare

Reputation: 351616

Yes, this is effectively allowing you to re-use the same array and not re-allocate a new one each time.

However, since you are returning an array (which is mutable) you cannot guarantee that the caller of your method isn't modifying the array once you return it. It would be much better to return an IEnumerable<T> or a ReadOnlyCollection<T> so that you can be sure that no one is modifying the collection besides you. (This has nothing to do with performance - just something I thought would be important to point out.)

Upvotes: 6

pmarflee
pmarflee

Reputation: 3428

You could also initialize the variable with Enumerable.Range.

static int[] MonthDays = Enumerable.Range(1, 31).ToArray();

Upvotes: 4

Hans Doggen
Hans Doggen

Reputation: 1826

If you don't mind that callers can change the array and mess up other callers, then this an optimization, but tricky in my opinion.

Upvotes: 0

Lee
Lee

Reputation: 144206

Yes, it will only be created once, although be aware that if any code decides to modify the return value then it will be reflected for all other callers, so it may be preferable to return a new copy each time. Alternatively you could only expose IEnumerable<int> instead. Incidently, you could also replace the initialiser to:

static int[] MonthDays = Enumerable.Range(1, 31).ToArray()

Upvotes: 5

JaredPar
JaredPar

Reputation: 755259

It is true that you will save allocating a new array every time GetListOfAllDaysForMonths is called by using the static variable. What's ambiguous is whether or not this is a worthwhile change for your code. That answer is highly dependent on the use cases of this method within your application. Only a profiler can tell you if this is a worthwhile savings.

Upvotes: 3

Related Questions