none
Proper use of Static List<t> RRS feed

  • Question

  • I will add an item to an EF data table and I want to look up the dept based on the item name in a 7000 record table of item names and their department.  Right now I'm do a "lookup" of sorts each time an item is added:

    internal List<DeptItem> GetDeptItems()
            {
                Entities pse = new Entities();
                List<DeptItem> deptItems = new List<DeptItem>();
                var DeptItems = from di in pse.DeptItems                            
                                select di;
                foreach (DeptItem deptItem in DeptItems)
                {
                    deptItems.Add(deptItem);
                }
                return deptItems;
            }

    Since this list rarely ever changes I'd like to do the heavy lifting once - possibly on app load.  Put all the data into a globally accessed List variable and just use the variable List as many time as I need to when items are added.

    I'm a little green with C# and I tried to use this

    List<DeptItem> deptItems = new List<DeptItem>();

    But where I had it  won't let me then say deptItems = GetDeptItems();

    Should this be in it's own static class or something?  

    Wednesday, October 2, 2019 8:23 PM

Answers

  • Greetings MA.

    There are umpteen ways to deal with this. The right way will depend on the full context.

    Karen's suggestion is a possibility. Here's another.

    List<DeptItem> deptItems;
    
    // I suspect this is what you tried and didn't work.
    // deptItems = GetDeptItems();
    
    
    internal List<DeptItem> GetDeptItems()
    {
        // Only do the heavy lifting if we haven't done it before.
        if(deptItems == null)
        {
           Entities pse = new Entities();
           deptItems = new List<DeptItem>();
           var DeptItems = from di in pse.DeptItems                            
                                select di;
           foreach (DeptItem deptItem in DeptItems)
           {
               deptItems.Add(deptItem);
           }
        }
        return deptItems;
    }


    • Marked as answer by Mimosa Arts Friday, October 4, 2019 1:02 PM
    Wednesday, October 2, 2019 11:38 PM
  • If you really declare your deptItems as "static List<DeptItem>", your GetDeptItems need to be static method as well, and you'll want to set it at the static constructor of the containing static class.
    • Marked as answer by Mimosa Arts Friday, October 4, 2019 1:02 PM
    Thursday, October 3, 2019 1:56 AM
    Answerer
  • Ante,

    But it's still doing the heavy lifting during adding the first SaleItem as the SaleItem.Department needs to be populated before adding the item to the database. Your solution keeps it from doing the population of the List each time but I was trying to get the list of GetItems populated BEFORE the first item is ever scanned so the list would be an accessible List to use so processing the first item would be faster.

    All this code is running in an internal class name Utilities.

    Thanks!!

    Harry

    • Marked as answer by Mimosa Arts Friday, October 4, 2019 1:02 PM
    Thursday, October 3, 2019 1:19 PM
  • Ideally you should handle this in your service layer perhaps using redis or similar to cache the results. That is what it is designed for. If that isn't an option then at least consider putting it into your business layer so your data layer can retrieve the data on demand (in case the data changes for example). Thus your "list" would be somewhere that the business layer has access to. This could be a simple static field in the class or it could be a completely different type depending upon your needs. In web apps, for example, we would tend to use Session to store data so the caching is independent of the actual service. This also makes it easier when you want to cache something else. For non-web apps you can use MemoryCache. 

    //Your service class
    public IEnumerable<DepItem> GetItems ()
    {
       //Have we loaded it yet
       var items = MemoryCache.Default.Get("DepItems") as IEnumerable<DepItem>;
       if (items == null)
       {
          //Load it from data layer
          items = GetDataFromDatabase();
    
          //Cache it
          var cacheItem = new CacheItem("DepItems", items);
          var policy = new CacheItemPolicy();
          MemoryCache.Default.Add(cacheItem, policy);
       };
    
       return items;
    }

    Here I'm using the default memory cache but unfortunately this makes testing your code harder because all your unit tests will rely on the cache so they may interact. The preferred approach would be for your layer to accept the cache to use as a parameter (injected via DI) so your unit tests can run in isolation but that is beyond this post.

    You mentioned you want to trigger it at app load. Personally I would recommend against this for almost all cases except data that is absolutely required at the very start every time the app runs. In all other cases lazy loading increases app performance and doesn't waste time loading stuff that may never be needed. That is what the above code does. You are always going to take the hit for loading the data. Whether that happens at app startup or when the user requests the data for the first time isn't relevant. The issue is that as you add more stuff to "warm load" at startup the slower your app loads. This has a negative impact on user perception of app speed. 

    Still if you want to trigger this at app startup then in your init logic for your app just call the method to get the items. You'll throw the items away but it will have already cached the data. For a winform app this is probably most appropriate after you've inited your DB but before you've loaded your main window.

    Note also that in my code example I haven't set any policies on the code. One advantage of using MemoryCache is that you can have things in the cache expire after a set/sliding time to reduce memory usage. Additionally you can set up callbacks to allow your cache to be reset when something changes (like a database table).


    Michael Taylor http://www.michaeltaylorp3.net

    • Marked as answer by Mimosa Arts Friday, October 4, 2019 1:02 PM
    Thursday, October 3, 2019 2:29 PM
    Moderator
  • Again, you are not doing optimal programming here, becuase you do not recognize that you are reading data from the DB twice, which is one time the data is read on the query and the second time that same data is being read from the DB again on each iteration in the loop. 

    Close the EF connection and disconnect from the DB before you start looping on the data in the collection.

    And what's with all the static stuff? Maybe you should consider using an IoC and dependency injection.


    • Edited by DA924x Thursday, October 3, 2019 6:08 PM
    • Marked as answer by Mimosa Arts Friday, October 4, 2019 1:02 PM
    Thursday, October 3, 2019 6:06 PM

All replies

  • Something like a Singleton would be one option which can be expanded upon.

    public sealed class DepartmentReference
    {
        private static readonly Lazy<DepartmentReference> Lazy =
            new Lazy<DepartmentReference>(() =>
                new DepartmentReference());
    
        public static DepartmentReference Instance => Lazy.Value;
    
        public List<DeptItem> DepartmentList = new List<DeptItem>();
        private void CreateList()
        {
            DepartmentList = new List<DeptItem>();
            // populate the list here
        }
    
        /// <summary>
        /// Instantiate List
        /// </summary>
        private DepartmentReference()
        {
            CreateList();
        }
    }

    Access the list


    Please remember to mark the replies as answers if they help and unmarked them if they provide no help, this will help others who are looking for solutions to the same or similar problem. Contact via my Twitter (Karen Payne) or Facebook (Karen Payne) via my MSDN profile but will not answer coding question on either.

    NuGet BaseConnectionLibrary for database connections.

    StackOverFlow
    profile for Karen Payne on Stack Exchange

    Wednesday, October 2, 2019 8:34 PM
    Moderator
  • Greetings MA.

    There are umpteen ways to deal with this. The right way will depend on the full context.

    Karen's suggestion is a possibility. Here's another.

    List<DeptItem> deptItems;
    
    // I suspect this is what you tried and didn't work.
    // deptItems = GetDeptItems();
    
    
    internal List<DeptItem> GetDeptItems()
    {
        // Only do the heavy lifting if we haven't done it before.
        if(deptItems == null)
        {
           Entities pse = new Entities();
           deptItems = new List<DeptItem>();
           var DeptItems = from di in pse.DeptItems                            
                                select di;
           foreach (DeptItem deptItem in DeptItems)
           {
               deptItems.Add(deptItem);
           }
        }
        return deptItems;
    }


    • Marked as answer by Mimosa Arts Friday, October 4, 2019 1:02 PM
    Wednesday, October 2, 2019 11:38 PM
  • If you really declare your deptItems as "static List<DeptItem>", your GetDeptItems need to be static method as well, and you'll want to set it at the static constructor of the containing static class.
    • Marked as answer by Mimosa Arts Friday, October 4, 2019 1:02 PM
    Thursday, October 3, 2019 1:56 AM
    Answerer
  • You do know that since you never disconnected from EF after the query, the loop is iterating over each item in the list that is sending EF back to the database to read each item again. So, that's 14,000 times you read the data. 

    And why does GetDeptItems() need to be in its own class, since you're not implementing SoC in general for the solution, like having a data access layer in the solution? 

    https://en.wikipedia.org/wiki/Separation_of_concerns

    • Marked as answer by Mimosa Arts Friday, October 4, 2019 1:02 PM
    • Unmarked as answer by Mimosa Arts Friday, October 4, 2019 1:02 PM
    Thursday, October 3, 2019 7:55 AM
  • Ante,

    But it's still doing the heavy lifting during adding the first SaleItem as the SaleItem.Department needs to be populated before adding the item to the database. Your solution keeps it from doing the population of the List each time but I was trying to get the list of GetItems populated BEFORE the first item is ever scanned so the list would be an accessible List to use so processing the first item would be faster.

    All this code is running in an internal class name Utilities.

    Thanks!!

    Harry

    • Marked as answer by Mimosa Arts Friday, October 4, 2019 1:02 PM
    Thursday, October 3, 2019 1:19 PM
  • Ideally you should handle this in your service layer perhaps using redis or similar to cache the results. That is what it is designed for. If that isn't an option then at least consider putting it into your business layer so your data layer can retrieve the data on demand (in case the data changes for example). Thus your "list" would be somewhere that the business layer has access to. This could be a simple static field in the class or it could be a completely different type depending upon your needs. In web apps, for example, we would tend to use Session to store data so the caching is independent of the actual service. This also makes it easier when you want to cache something else. For non-web apps you can use MemoryCache. 

    //Your service class
    public IEnumerable<DepItem> GetItems ()
    {
       //Have we loaded it yet
       var items = MemoryCache.Default.Get("DepItems") as IEnumerable<DepItem>;
       if (items == null)
       {
          //Load it from data layer
          items = GetDataFromDatabase();
    
          //Cache it
          var cacheItem = new CacheItem("DepItems", items);
          var policy = new CacheItemPolicy();
          MemoryCache.Default.Add(cacheItem, policy);
       };
    
       return items;
    }

    Here I'm using the default memory cache but unfortunately this makes testing your code harder because all your unit tests will rely on the cache so they may interact. The preferred approach would be for your layer to accept the cache to use as a parameter (injected via DI) so your unit tests can run in isolation but that is beyond this post.

    You mentioned you want to trigger it at app load. Personally I would recommend against this for almost all cases except data that is absolutely required at the very start every time the app runs. In all other cases lazy loading increases app performance and doesn't waste time loading stuff that may never be needed. That is what the above code does. You are always going to take the hit for loading the data. Whether that happens at app startup or when the user requests the data for the first time isn't relevant. The issue is that as you add more stuff to "warm load" at startup the slower your app loads. This has a negative impact on user perception of app speed. 

    Still if you want to trigger this at app startup then in your init logic for your app just call the method to get the items. You'll throw the items away but it will have already cached the data. For a winform app this is probably most appropriate after you've inited your DB but before you've loaded your main window.

    Note also that in my code example I haven't set any policies on the code. One advantage of using MemoryCache is that you can have things in the cache expire after a set/sliding time to reduce memory usage. Additionally you can set up callbacks to allow your cache to be reset when something changes (like a database table).


    Michael Taylor http://www.michaeltaylorp3.net

    • Marked as answer by Mimosa Arts Friday, October 4, 2019 1:02 PM
    Thursday, October 3, 2019 2:29 PM
    Moderator
  • namespace SaleSuggestions
    {
        internal class Utilities
        {
            private List<string> portList = new List<string>();
            private Images images = new Images();
    //*************************************************************************
           static List<DeptItem> deptItems = new List<DeptItem>();
            deptItems = GetDeptItems();
    //*************************************************************************
            static List<DeptItem> GetDeptItems()
            {
                // Only do the heavy lifting if we haven't done it before.
                if (deptItems == null)
                {
                    Entities pse = new Entities();
                    deptItems = new List<DeptItem>();
                    var DeptItems = from di in pse.DeptItems
                                    select di;
                    foreach (DeptItem deptItem in DeptItems)
                    {
                        deptItems.Add(deptItem);
                    }
                }
                return deptItems;
            }
            //*******************************

    The  deptItems = GetDeptItems(); shows error that method must have a return type. It does

    ?????

    Harry

    Thursday, October 3, 2019 2:43 PM
  • Again, you are not doing optimal programming here, becuase you do not recognize that you are reading data from the DB twice, which is one time the data is read on the query and the second time that same data is being read from the DB again on each iteration in the loop. 

    Close the EF connection and disconnect from the DB before you start looping on the data in the collection.

    And what's with all the static stuff? Maybe you should consider using an IoC and dependency injection.


    • Edited by DA924x Thursday, October 3, 2019 6:08 PM
    • Marked as answer by Mimosa Arts Friday, October 4, 2019 1:02 PM
    Thursday, October 3, 2019 6:06 PM
  • This is what I ultimately did - in the form load of the main page added this call

     utilities.LoadItemDepts();

    then this in Utilities class

      public void LoadItemDepts()
            {
                allDeptItems = GetDeptItems();
            }

    and then this

    internal List<DeptItem> GetDeptItems() { Entities pse = new Entities(); List<DeptItem> deptItems = new List<DeptItem>(); var DeptItems = from di in pse.DeptItems select di; foreach (DeptItem deptItem in DeptItems) { deptItems.Add(deptItem); } return deptItems; }

      static internal List<DeptItem> allDeptItems = new List<DeptItem>();

    Really appreciate all the great input.  Learned a lot and thankful for such a great community!!

    Friday, October 4, 2019 1:00 PM