2
\$\begingroup\$

I am working on my template web api project.

I just wanna hear any suggestion how to improve this code. I heard that repository pattern is anti pattern so I want to make code without it. Any opinion will be great :D

Service

public class TemplateService : ITemplateService
{
 private readonly ContextTemplate context;
 public TemplateService(ContextTemplate context)
 {
 this.context = context;
 }
 public int AddTemplate(TemplateInput input)
 {
 var template = new TemplateModel()
 {
 Name = input.Name,
 CreatedDate = DateTime.UtcNow
 };
 context.TemplateModel.Add(template);
 context.SaveChanges();
 return template.Id;
 }
 public int UpdateTemplate(TemplateInput input)
 {
 var template = GetTemplate(input.Id);
 if (template != null) {
 template.Name = input.Name;
 template.ModifiedDate = DateTime.UtcNow;
 context.Update(template);
 context.SaveChanges();
 }
 return (int)template?.Id;
 }
 public bool ArchiveTemplate(int templateId)
 {
 var isArchiveSuccessfull = false;
 var template = GetTemplate(templateId);
 if (template != null)
 {
 template.IsArchived = true;
 isArchiveSuccessfull = true;
 context.Update(template);
 context.SaveChanges();
 }
 return isArchiveSuccessfull;
 }
 public IEnumerable<TemplateModelDto> GetTemplates()
 {
 return Mapper.Map<List<TemplateModel>, List<TemplateModelDto>>(this.context.TemplateModel.Where(x=>!x.IsArchived).ToList());
 }
 public TemplateModel GetTemplate(int templateId)
 {
 var template = context.TemplateModel.FirstOrDefault(x => x.Id == templateId);
 return template;
 }

Controller

 [Route("api/[controller]")]
 public class TemplatesController : Controller
 {
 private readonly ITemplateService templateService;
 private readonly ILogger logger;
 public TemplatesController(ITemplateService templateService, ILogger<TemplatesController> logger)
 {
 this.templateService = templateService;
 this.logger = logger;
 }
 // GET api/templates
 [HttpGet]
 public IEnumerable<TemplateModelDto> Get() => templateService.GetTemplates();
 // GET api/templates/5
 [HttpGet("{templateId}")]
 public TemplateModel GetTemplate(int templateId) => templateService.GetTemplate(templateId);
 // POST api/templates
 [HttpPost]
 public int Post([FromBody]TemplateInput input) => templateService.AddTemplate(input);
 // PUT api/templates/5
 [HttpPut("{templateId}")]
 public int Put(int templateId, [FromBody]TemplateInput input) => templateService.UpdateTemplate(input);
 // DELETE api/templates/5
 [HttpDelete("{templateId}")]
 public Boolean ArchiveTemplate(int templateId) => templateService.ArchiveTemplate(templateId);
}
t3chb0t
44.6k9 gold badges84 silver badges190 bronze badges
asked Feb 6, 2018 at 22:47
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

You have abstracted away any concrete dependencies from the controller. I say that is well done for decoupling the controller and would allow better testability of the controller.

ContextTemplate is assumed to be DbContext derived, which already implements Unit of Work and Repository pattern so there is really no use in re-implementing the pattern.

IMO I see no immediate problems with the above code apart from the non use of the explicitly injected ILogger in the controller, which can be removed if there is no use for it in the controller. Logging can be considered a cross-cutting concern in asp.net-core and handled external to the controller.

answered Feb 7, 2018 at 7:35
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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.