Can this async method be improved or simplified

I was tasked to write process that initially seemed like a straight forward thing. “Fetch data from database, create a cache object, and assign this data to it. Use existing class to model this after.” That’s it in a nutshell. The ‘existing’ class did not do any caching, just an async call across layers to the database. The purpose the async method is prevent the app from waiting for this data to load. I should mention this is custom app that run as a plug-in to the MS Excel (Don’t ask!), and so it’s a WPF app. As such, caching of larger data is done with a use of Dictionaries rather than using System.Runtime.Caching library. Originally I used a dictionary like this: public async Task<Dictionary<projectId, ProjectFeatureIndicatorMetadata>> GetProjectFeatureIndicatorsStatusAsync(Guid projectId) but the tech lead decided not to use a Dictionary due unnecessary memory usage.

So what I’m looking to gain from this post is constructive comments to see if this is overkill, can it be simplified, or improved, etc. Bottom line is, code works I just want to see if it can be improved. If you decide to down vote it, I would ask to leave comment as to why.

Doing to start with the data layer. So here the ReaderAsync reads the data set and assigns it to the ProjectFeatureIndicatorMetadata class:

public async Task<ProjectFeatureIndicatorMetadata> GetProjectFeatureIndicatorsStatusAsync(Guid projectId)
{
    ProjectFeatureIndicatorMetadata result = null;

    await GetSpHelper().ExecuteReaderAsync(
       cancellationToken: CancellationToken.None,
       spName: "ITV.usp_ProjectFeatureIndicators_GetByProjectId",
       parmsDg: parms => {
       parms.AddWithValue("@ProjectId:", projectId);
       },
       methodDg: async (reader, ct) =>
       {
          while (await reader.ReadAsync(ct))
          {
              result = new ProjectFeatureIndicatorMetadata(
                  Convert.ToBoolean(reader["SprcIncludesInd"]),
                  Convert.ToBoolean(reader["SprcVariablesInd"]),
                  Convert.ToBoolean(reader["SprcFlowInd"]),
                  Convert.ToBoolean(reader["SprcGenerationInd"]));
          }
          return true;
       });
   return result;
}

Then I have the this code in one of the business layers. I only included pertinent code:

public class ItvCacheFeatureFlagManager : IitvCacheFeatureFlagManager
{
   public static ProjectFeatureIndicatorMetadata _featureIndicatorFlagCache = 
             new ProjectFeatureIndicatorMetadata(true,true,true,true);

   public virtual async Task<ProjectFeatureIndicatorMetadata> GetProjectFeatureIndicatorMetadata(Guid projectId)
    {
        using (new WriteLock(_lock))
        {
            if (_featureIndicatorFlagCache != null)
            {
                return _featureIndicatorFlagCache;
            }
            else
            {
                var dbresult = await GetProjectFeatureIndicatorsStatusAsync(projectId);                    
                return dbresult;
            }
        }
    }
}

The type ProjectFeatureIndicatorMetadata is like model class and this is where I was not certain if I even need this:

public class ProjectFeatureIndicatorMetadata
{
    public ProjectFeatureIndicatorMetadata(
        bool SprcIncludesInd,
        bool sprcVariablesInd,
        bool sprcFlowInd,
        bool sprcGenerationInd)
    {
        SprcIncludesInd = sprcIncludesInd;
        SprcVariablesInd = sprcVariablesInd;
        SprcFlowInd = sprcFlowInd;
        SprcGenerationInd = sprcGenerationInd;
    }       

    public bool SprcIncludesInd { get; set; }
    public bool SprcVariablesInd { get; set; }
    public bool SprcFlowInd { get; set; }
    public bool SprcGenerationInd { get; set; }
}

Update: I refactored some of the code and included some of the startup code as it was suggested below.

 //Staring class:
 public class ClientUserInterface : IClientUserInterface
 {
    [NotNull]
    private ProjectData Data
    {
       [Pure, DebuggerStepThrough]
       get { return ProjectDataProvider.ProjectData; }
    }

    private void OpenPublisher(ProjectIdentifier project)
    {
       Task.Run(async () => await Data.GetProjectFeatureIndicatorMetadata(project));
    }
  }

  public class ProjectData : DependencyObject, INotifyPropertyChanged, IDisposable
  {
     // the green squidgy under _featureManger says "Field 'ProjectData._featureManger' 
     // is never assigned to, and will have its default value null
     private readonly IitvCacheFeatureFlagManager _featureManger;

     public async Task<ProjectFeatureIndicatorMetadata>
        GetProjectFeatureIndicatorMetadata(ProjectIdentifier projectId)
     {
        return await _featureManger.GetProjectFeatureIndicatorMetadata(projectId);
     }
  }

  public interface IitvCacheFeatureFlagManager
  {
     Task<ProjectFeatureIndicatorMetadata> 
         GetProjectFeatureIndicatorMetadata(Guid projectId);
  }

  public class ItvCacheFeatureFlagManager : IitvCacheFeatureFlagManager
  {
     private readonly ItvCacheFeatureFlagProvider _provider;
     private readonly ReaderWriterLockSlim _lock = 
          new ReaderWriterLockSlim();

     public static ProjectFeatureIndicatorMetadata _featureIndicatorFlagCache = 
         new ProjectFeatureIndicatorMetadata();

     public ItvCacheFeatureFlagManager( SqlConnectionFactory connectionFactory )
     {
        _provider = new ItvCacheFeatureFlagProvider(connectionFactory);
     }

     public virtual async Task<ProjectFeatureIndicatorMetadata>
         GetProjectFeatureIndicatorMetadata(Guid projectId)
     {
         using (new WriteLock(_lock))
         {
            if (_featureIndicatorFlagCache != null)
            {
               return _featureIndicatorFlagCache;
            }
            else
            {
              var dbresult = await GetProjectFeatureIndicatorsStatusAsync(projectId);                    
              return dbresult;
            }
        }
     }

     public virtual async Task<ProjectFeatureIndicatorMetadata> 
        GetProjectFeatureIndicatorsStatusAsync(Guid projectId)
     {
       var dbresult = await _provider.GetProjectFeatureIndicatorsStatusAsync(projectId);            
       return dbresult;
     }
 }

 public class ItvCacheFeatureFlagProvider : IitvCacheFeatureFlagProvider
 {
    private SqlConnectionFactory ConnectionFactory { get; }

    private SqlSpHelper GetSpHelper() => new SqlSpHelper(ConnectionFactory);

    public ItvCacheFeatureFlagProvider(SqlConnectionFactory connectionFactory)
    {
       ConnectionFactory = connectionFactory;
    }

    public async Task<ProjectFeatureIndicatorMetadata>
       GetProjectFeatureIndicatorsStatusAsync(Guid projectId)
    {
       ProjectFeatureIndicatorMetadata result = null;

       await GetSpHelper().ExecuteReaderAsync(
       cancellationToken: CancellationToken.None,
       spName: "ITV.usp_ProjectFeatureIndicators_GetByProjectId",
       parmsDg: parms => {
             parms.AddWithValue("@ProjectId:", projectId);
       },
       methodDg: async (reader, ct) =>
       {
          while (await reader.ReadAsync(ct))
          {
             result.SprcIncludesInd = Convert.ToBoolean(reader["SprcIncludesInd"]);
             result.SprcVariablesInd = Convert.ToBoolean(reader["SprcVariablesInd"]);
             result.SprcGenerationInd = Convert.ToBoolean(reader["SprcGenerationInd"]);
             result.SprcFlowInd = Convert.ToBoolean(reader["SprcFlowInd"]);
          }  
          return true;
       });
       return result;
    }
 }

public class ProjectFeatureIndicatorMetadata
{
    public ProjectFeatureIndicatorMetadata()
    {
    }       

    public bool? SprcIncludesInd { get; set; }
    public bool? SprcVariablesInd { get; set; }
    public bool? SprcFlowInd { get; set; }
    public bool? SprcGenerationInd { get; set; }
}

Does this code work? The _featureIndicatorFlagCache is set right away and is never cleared out. So the check if it’s null will always be false and send back just he default and never execute GetProjectFeatureIndicatorsStatusAsync.

I assume you want to cache the call to GetProjectFeatureIndicatorsStatusAsync but you won’t be able to store that in one property unless it’s a dictionary as I assume the data it returns is based on projectId.

If you want an example of a simple caching you can use the Lazy<Task<>> in the dictionary. The Lazy will only execute once and return the same task back to all the callers. I would just use the ConcurrentDictionary as not have to deal with the locking myself.

public static ConcurrentDictionary<Guid, Lazy<Task<ProjectFeatureIndicatorMetadata>>> _cache =
    new ConcurrentDictionary<Guid, Lazy<Task<ProjectFeatureIndicatorMetadata>>>();

The the cache method would look something like this

public virtual Task<ProjectFeatureIndicatorMetadata> GetProjectFeatureIndicatorMetadata(
    Guid projectId)
{
    // This is optional just removes any tasks that have been faulted to re-execute 
    if (_cache.TryGetValue(projectId, out var result))
    {
        // IsFaulted will be false while task is in flight - this is what we want and don't want to block on it
        if (result.Value.IsFaulted)
        {
            // cast as dictionary to get remove that also checks the value along with the key
            var cacheDictionary = (IDictionary<Guid, Lazy<Task<ProjectFeatureIndicatorMetadata>>>) _cache;
            cacheDictionary.Remove(
                new KeyValuePair<Guid, Lazy<Task<ProjectFeatureIndicatorMetadata>>>(projectId, result));
            result = null;
        }
    }

    if (result == null)
    {
        result = _cache.GetOrAdd(projectId, id => new Lazy<Task<ProjectFeatureIndicatorMetadata>>(GetProjectFeatureIndicatorsStatusAsync(id)));
    }

    return result.Value;
}