0

Some one could you please help me out to replace foreach with lambda from below two methods or anything else optimization will be appreciated.

/// <summary>
/// This method will find and remove the Template entries that has no person entris
/// from List<Template> Templates
/// </summary>
/// <param name="Persons"></param>
/// <param name="Templates"></param>
public bool ClearOrphnedIDs(List<Person> Persons, List<Template> Templates)
{
    bool isClearComplete = false;
    try
    {
        if (Persons != null && Templates != null)
        {
            List<string> OrphnedTemplatesNeedToIgnore = new List<string>();
            foreach (Template template in Templates)
            {
                string personID = Persons.Find(p => p.PersonID == template.PersonID).PersonID;
                if (string.IsNullOrEmpty(personID) && !OrphnedTemplatesNeedToIgnore.Contains(personID))
                {
                        OrphnedTemplatesNeedToIgnore.Add(template.PersonID);
                        DataSyncLog.Warn(string.Format("Templates with personID {0} is orphned (has no person entry) in DB", template.PersonID));
                }

            }
            if (OrphnedTemplatesNeedToIgnore.Count > 0)
                Templates.RemoveAll(t=> OrphnedTemplatesNeedToIgnore.Contains(t.PersonID));
            isClearComplete = true;
        }
    }
    catch (Exception ex)
    {
        DataSyncLog.Debug(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType + "::" + System.Reflection.MethodBase.GetCurrentMethod().ToString() + " :: " + ex.Message + " :: " + ex.StackTrace);
    }
    return isClearComplete;
}

/// <summary>
/// This method will Find and remove the Person and template entries that has 
/// zero or odd number of templates. from List<Person> Persons and List<Template> Templates
/// </summary>
/// <param name="Persons"></param>
/// <param name="Templates"></param>
public bool ClearInconsistantIDs(List<Person> Persons, List<Template> Templates)
{
    bool isClearComplete = false;
    try
    {
        if (Persons != null && Templates != null)
        {
            List<string> personNeedtoIgnoreAlongWithItsTemplates = new List<string>();
            foreach (Person person in Persons)
            {
                int templatesCount = Templates.FindAll(t => t.PersonID == person.PersonID).Count;
                if (templatesCount == 0 || templatesCount % 2 != 0)
                {
                    personNeedtoIgnoreAlongWithItsTemplates.Add(person.PersonID);
                    if (templatesCount == 0)
                        DataSyncLog.Warn(string.Format("Person with Registration No: {0} and personID {1} has no Templates in DB. Templates Count: {2}", person.RegistrationNO, person.PersonID, templatesCount));
                    else
                        DataSyncLog.Warn(string.Format("Person with Registration No: {0} and personID {1} has inconsistent data (Templates) in DB. Templates Count: {2}", person.RegistrationNO, person.PersonID, templatesCount));

                }
            }
            if (personNeedtoIgnoreAlongWithItsTemplates.Count > 0)
            {
                Templates.RemoveAll(t => personNeedtoIgnoreAlongWithItsTemplates.Contains(t.PersonID));
                Persons.RemoveAll(p => personNeedtoIgnoreAlongWithItsTemplates.Contains(p.PersonID));
            }
            isClearComplete = true;
        }

    }
    catch (Exception ex)
    {
        DataSyncLog.Debug(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType + "::" + System.Reflection.MethodBase.GetCurrentMethod().ToString() + " :: " + ex.Message + " :: " + ex.StackTrace);
    }
    return isClearComplete;
}
Improve this question
10
  • 11
    Why do you think a lambda would be an optimization? Commented Sep 6, 2013 at 7:05
  • 4
    Did you try anything yourself? If not, why not? If yes, please show it and tell us what is not working. Furthermore, please only show the code relevant to your question Commented Sep 6, 2013 at 7:05
  • codereview.stackexchange.com? Commented Sep 6, 2013 at 7:07
  • geekswithblogs.net/BlackRabbitCoder/archive/2010/04/23/… Commented Sep 6, 2013 at 7:11
  • Reshaper can do this for you sometimes :) Commented Sep 6, 2013 at 7:35

3 Answers 3

Reset to default
4

I don't think you can do better with lambda, but you definitely can do better using HashSet<string> instead of List<string> and making another HashSet<string> for PersonID values from Persons parameter:

public bool ClearOrphnedIDs(List<Person> Persons, List<Template> Templates)
{
    bool isClearComplete = false;
    try
    {
        if (Persons != null && Templates != null)
        {
            var personsSet = new HashSet<string>(Persons.Select(p => p.PersonId));
            var orphnedTemplatesNeedToIgnore = new HastSet<string>();
            foreach (var template in Templates)
            {
                if (!personsSet.Contains(template.PersonID) && !orphnedTemplatesNeedToIgnore.Contains(personID))
                {
                        orphnedTemplatesNeedToIgnore.Add(template.PersonID);
                        DataSyncLog.Warn(string.Format("Templates with personID {0} is orphned (has no person entry) in DB", template.PersonID));
                }

            }
            if (orphnedTemplatesNeedToIgnore.Count > 0)
                Templates.RemoveAll(t => orphnedTemplatesNeedToIgnore.Contains(t.PersonID));
            isClearComplete = true;
        }
    }
    catch (Exception ex)
    {
        DataSyncLog.Debug(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType + "::" + System.Reflection.MethodBase.GetCurrentMethod().ToString() + " :: " + ex.Message + " :: " + ex.StackTrace);
    }
    return isClearComplete;
}
Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

Just reading about HashSet , Is it required to check !personsSet.Contains(template.PersonID) ??? or will can directly use orphnedTemplatesNeedToIgnore.Add(template.PersonID); ??
foreach (var template in Templates) { orphnedTemplatesNeedToIgnore.Add(template.PersonID); DataSyncLog.Warn(string.Format("Templates with personID {0} is orphned (has no person entry) in DB", template.PersonID)); } I think this will do .
So, @MarcinJuraszek you say this code is fine. Nothing need to optimize right? though if i not use the HashSet.
@surajsingh logic is if the template.PersonID does not contains in personsSet then add this to the ignorelist
@Rezoan Well you should read full article before forwarding it ,and try something do not depend upon what you read .HashSet do not allows insertion of duplicate values .try this HashSet<int> trialtwo = new HashSet<int>(); for (int i = 0; i <= 10; i++) { trialtwo.Add(i); } bool ti = Convert.ToBoolean(trialtwo.Add(5));
|
1

I would define an extension method ExceptBy

var result = Templates.ExceptBy(Persons, t => t.PersonID, p => p.PersonID)
                      .ToList();

public static IEnumerable<TSource> ExceptBy<TSource, TKey, TDest>(
        this IEnumerable<TSource> first,
        IEnumerable<TDest> second,
        Func<TSource, TKey> keySelector1,
        Func<TDest, TKey> keySelector2)
{
    HashSet<TKey> keys = new HashSet<TKey>(second.Select(keySelector2));

    return first.Where(k => keys.Add(keySelector1(k)));
}
Improve this answer

Comments

0

Guys How about this,

/// <summary>
    /// This method will find and remove the Template entries that has no person entris
    /// from List<Template> Templates
    /// </summary>
    /// <param name="Persons"></param>
    /// <param name="Templates"></param>
    public bool ClearOrphnedIDs(List<Person> Persons, List<Template> Templates)
    {
        bool isClearComplete = false;
        try
        {
            if (Persons != null && Templates != null)
            {
                HashSet<string> personsSet = new HashSet<string>(Persons.Select(p => p.PersonID));
                Templates.RemoveAll(t => !personsSet.Contains(t.PersonID));
                isClearComplete = true;
            }
        }
        catch (Exception ex)
        {
            DataSyncLog.Debug(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType + "::" + System.Reflection.MethodBase.GetCurrentMethod().ToString() + " :: " + ex.Message + " :: " + ex.StackTrace);
        }
        return isClearComplete;
    }

    /// <summary>
    /// This method will Find and remove the Person and template entries that has 
    /// zero or odd number of templates. from List<Person> Persons and List<Template> Templates
    /// </summary>
    /// <param name="Persons"></param>
    /// <param name="Templates"></param>
    public bool ClearInconsistantIDs(List<Person> Persons, List<Template> Templates)
    {
        bool isClearComplete = false;
        try
        {
            if (Persons != null && Templates != null)
            {
                HashSet<string> inconsistantIDs = new HashSet<string>(
                  Persons.Select(p => p.PersonID).Where(p =>
                  {
                      var count = Templates.Count(t => t.PersonID == p);
                      return count == 0 || count % 2 != 0;
                  }
                  ));

                if (inconsistantIDs.Count > 0)
                {
                    Templates.RemoveAll(t => inconsistantIDs.Contains(t.PersonID));
                    Persons.RemoveAll(p => inconsistantIDs.Contains(p.PersonID));
                }
                isClearComplete = true;
            }

        }
        catch (Exception ex)
        {
            DataSyncLog.Debug(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType + "::" + System.Reflection.MethodBase.GetCurrentMethod().ToString() + " :: " + ex.Message + " :: " + ex.StackTrace);
        }
        return isClearComplete;
    }
Improve this answer

Comments

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.