1
\$\begingroup\$

I have belowe tables, for Map string[] Tags from ViewModel to Model I'm using Below codes :

  public class Tag:BaseEntity
{
    public string Name { get; set; }
    public virtual ICollection<Blog.BlogPost> BlogPosts { get; set; }
    public virtual ICollection<News.NewsItem> NewsItems { get; set; }

}
 public class BlogPost : BaseContent, IAuditable
    {
    public virtual ICollection<Tag> Tags { get; set; }
}


public class NewsItem : BaseContent, IAuditable
    {
    public virtual ICollection<Tag> Tags { get; set; }

    public NewsItem()
    {
        Tags = new List<Tag>();
    }
}

In newsService I have two methods Insert ,Update to Add and Update NewsItem :

     public void Insert(AddNewsItemViewModel newsItemViewModel)
    {
        if (newsItemViewModel == null)
            throw new ArgumentNullException("newsItem");

        var model = Mapper.Map<NewsItem>(newsItemViewModel);

        if(newsItemViewModel.Tags.Any()) // Tags is `string[]`
        {
            var listOfActualTags = _tagService.GetTagsByName(newsItemViewModel.Tags); // return List<Tag> 
            var listOfActualTagNames = listOfActualTags.Select(x => x.Name.ToLower()).ToList();

            foreach (var tag in newsItemViewModel.Tags)
            {
                if (!listOfActualTagNames.Contains(tag.ToLowerInvariant().Trim()))
                {
                    Tag tagModel = new Tag { Name=tag};
                    _tagService.Insert(tagModel);
                    model.Tags.Add(tagModel);
                }
            }

            model.MetaKeywords =string.Join(",",newsItemViewModel.Tags);
        }
        _uow.MarkAsAdded(model);
    }

and for Update Method I have this Code :

        public void Update(EditNewsItemViewModel newsItemViewModel)
    {
        if (newsItemViewModel == null)
            throw new ArgumentNullException("newsItem");
        var model = GetById(newsItemViewModel.Id);
        model = Mapper.Map(newsItemViewModel, model);
        if (model.Tags != null && model.Tags.Any())
            model.Tags.Clear();

        var listOfActualTags = _tagService.GetTagsByName(newsItemViewModel.Tags); // return List<Tag>
        var listOfActualTagNames = listOfActualTags.Select(x => x.Name.ToLower()).ToList();

        foreach (var tag in newsItemViewModel.Tags)
        {
            Tag tagModel = new Tag { Name = tag };
            if (!listOfActualTagNames.Contains(tag.ToLowerInvariant().Trim()))
            {

                _tagService.Insert(tagModel);
                model.Tags.Add(tagModel);
            }
            else
                model.Tags.Add(tagModel);
        }

        model.MetaKeywords = string.Join(",", newsItemViewModel.Tags);

        _uow.MarkAsChanged(model);
    }

For BlogPost I have to repeat Above Code (for tag part) fro that , Is there a way to Optimize this ?

\$\endgroup\$
  • \$\begingroup\$ Mapper.Map<NewsItem>(newsItemViewModel); why are you mapping all models instead of simply showing what you have? Without mapping you could just save the updated data instead of translating it each time. \$\endgroup\$ – t3chb0t Jul 12 '17 at 13:53
3
\$\begingroup\$

Your Insert method has a bug - if a tag exists in a database, you won't add it to your model.Tags collection. Interesting enough, your Update method doesn't have that problem. After you fix that bug, you're left with almost identical part of the code that could also be reused by BlogPost logic.

Because it should be reused, it would be best to place it in your TagService and, after a few improvements, it would look like this:

public void AddTagsToList(ICollection<Tag> tagList, string[] tagNames)
{
  var listOfActualTags = GetTagsByName(tagNames);

  foreach (var tagName in tagNames)
  {
    var tag = listOfActualTags.FirstOrDefault(t => t.Name == tagName);

    if (tag == null)
    {
      tag = new Tag { Name = tagName };
      Insert(tag);
    }

    tagList.Add(tag);
  }
}

Your Insert method would become:

public void Insert(AddNewsItemViewModel newsItemViewModel)
{
  if (newsItemViewModel == null)
    throw new ArgumentNullException("newsItem");

  var model = Mapper.Map<NewsItem>(newsItemViewModel);

  if(newsItemViewModel.Tags.Any())
  {
    _tagService.AddTagsToList(model.Tags, newsItemViewModel.Tags);

    model.MetaKeywords = string.Join(",", newsItemViewModel.Tags);
  }

  _uow.MarkAsAdded(model);
}

And Update method would become:

public void Update(EditNewsItemViewModel newsItemViewModel)
{
  if (newsItemViewModel == null)
      throw new ArgumentNullException("newsItem");
  var model = GetById(newsItemViewModel.Id);
  model = Mapper.Map(newsItemViewModel, model);
  if (model.Tags != null && model.Tags.Any())
    model.Tags.Clear();

  if(newsItemViewModel.Tags.Any())
  {
    _tagService.AddTagsToList(model.Tags, newsItemViewModel.Tags);

    model.MetaKeywords = string.Join(",", newsItemViewModel.Tags);
  }

  _uow.MarkAsChanged(model);
}

As I said, you can reuse _tagService.AddTagsToList method for BlogPost as well by providing the Tags property as a first parameter and a list of tag names as a second parameter.

|improve this answer|||||
\$\endgroup\$
2
\$\begingroup\$
 if (!listOfActualTagNames.Contains(tag.ToLowerInvariant().Trim()))
        {

            _tagService.Insert(tagModel);
            model.Tags.Add(tagModel);
        }
        else
            model.Tags.Add(tagModel);

Can be changed to

 if (!listOfActualTagNames.Contains(tag.ToLowerInvariant().Trim()))
            _tagService.Insert(tagModel);

model.Tags.Add(tagModel);
|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.