MicroMan
MicroMan

Reputation: 2098

Design Pattern in C# to reduce Code Duplication looping through Class Lists

I have a rep with Media Items Each items can have several bookmarks and comments from different users.

When I come to update the repo I need to get all existing Comments and book marks and append the new comments and book marks, the code works, but i cant think of a clean way to do this generically that will reduce duplication, as more fields like this are planned

  public class MapValues
    {
        private readonly string _userName;
        private readonly Media _media;
        private readonly IMediaRepository _mediaRepo;

        public MapValues(string userName, Media media, IMediaRepository mediaRepo)
        {
            _userName = userName;
            _media = media;
            IMediaRepository = mediaRepo;
        }

        public async Task<Media> AppendExistingUserValues(Media updatedMedia, string userName)
        {
            var existingMedia = await _mediaRepo.GetByIdAsync(updatedMedia.Id);
            var existingBookmarks = GetBookmarksExcludingUser(userName, existingMedia);
            var existingComments = GetCommentsExcludingUser(userName, existingMedia);

            if (updatedMedia.Bookmarks.Any())
            {
                existingBookmarks.AddRange(updatedMedia.Bookmarks);
            }

            if (updatedMedia.Comments.Any())
            {
                existingComments.AddRange(updatedMedia.Comments);
            }

            updatedMedia.Bookmarks = existingBookmarks;
            updatedMedia.Comments = existingComments;
            return updatedMedia;
        }


        private List<MediaBookmarks> GetBookmarksExcludingUser(string userName, Media media)
        {
            var exisitingBookmarks = new List<MediaBookmarks>();
            if (media?.Bookmarks != null)
            {
                exisitingBookmarks.AddRange(media.Bookmarks
                    .Where(x => !string.Equals(x.Name, userName, StringComparison.CurrentCultureIgnoreCase)).ToList());
            }

            return exisitingBookmarks;
        }

        private List<MediaComments> GetCommentsExcludingUser(string userName, Media media)
        {
            var exisitingComments = new List<MediaComments>();
            if (media?.Bookmarks != null)
            {
                exisitingComments.AddRange(media.Comments
                    .Where(x => !string.Equals(x.Name, userName, StringComparison.CurrentCultureIgnoreCase)).ToList());
            }

            return exisitingComments;
        }
    }

    public class Media
    {
        public IEnumerable<MediaBookmarks> Bookmarks { get; set; }

        public IEnumerable<MediaComments> Comments { get; set; }
        public string Id { get; set; }
    }

    public class MediaBookmarks
    {
        public string Name { get; set; }
    }

    public class MediaComments
    {
        public string Name { get; set; }
    }

Upvotes: 0

Views: 93

Answers (2)

Filip Cordas
Filip Cordas

Reputation: 2561

From what I can tell this whole thing can be rewritten to

 public async Task<Media> AppendExistingUserValues2(Media updatedMedia, string userName)
    {
        var existingMedia = await _mediaRepo.GetByIdAsync(updatedMedia.Id);


        updatedMedia.Bookmarks = updatedMedia.Bookmarks
            .Union(existingMedia.Bookmarks.Where(x => !string.Equals(x.Name, userName, StringComparison.CurrentCultureIgnoreCase))).ToList();

        updatedMedia.Comments = updatedMedia.Comments
            .Union(existingMedia.Comments.Where(x => !string.Equals(x.Name, userName, StringComparison.CurrentCultureIgnoreCase))).ToList();

        return updatedMedia;
    }

I also like to use extender methods to make the string equals a bit more readable so with a helper.

public static class StringHelpers
    {
        public static bool IsSame(this string str1, string str2)
        {
            return string.Equals(str1, str2, StringComparison.CurrentCultureIgnoreCase);
        }
    }

you can just write.

public async Task<Media> AppendExistingUserValues2(Media updatedMedia, string userName)
        {
            var existingMedia = await _mediaRepo.GetByIdAsync(updatedMedia.Id);


            updatedMedia.Bookmarks = updatedMedia.Bookmarks
                .Union(existingMedia.Bookmarks.Where(x => x.Name.IsSame(userName))).ToList();

            updatedMedia.Comments = updatedMedia.Comments
                .Union(existingMedia.Comments.Where(x => x.Name.IsSame(userName))).ToList();

            return updatedMedia;
        }

Upvotes: 0

MikeH
MikeH

Reputation: 4395

If you create an interface (IMediaItem) then you can implement it for each of your specific classes (Bookmarks and Comments).

public interface IMediaItem //This could even be a class if required
{
    public string Name { get; set; }
}

 public class MediaBookmarks : IMediaItem 
{
  public string Name { get; set; }
}

public class MediaComments : IMediaItem 
{
  public string Name { get; set; }
}

Then in your GetBookmarksExcludingUser and GetCommentsExcludingUser could be combined into this:

private List<IMediaItem> GetMediaItemExcludingUser(string userName, IEnumerable<IMediaItem> sourceList)
    {
        var existingItems = new List<IMediaItem>();
        if (sourceList != null)
        {
            existingItems.AddRange(sourceList
                .Where(x => !string.Equals(x.Name, userName, StringComparison.CurrentCultureIgnoreCase)).ToList());
        }

        return existingItems;
    }

Call that method like this:

var existingBookmarks = GetMediaItemExcludingUser(userName, existingMedia?.Bookmarks).Cast<MediaBookmarks>();

Upvotes: 1

Related Questions