4
When developing a screen for a certain part of a system, I come across a classic situation of owning some ifs to determine what action should be taken. I then linked this case to examples of OCP infringement.
However, I saw that the screen is very cohesive and with a defined scope, where I do not see, and neither does the team see at some point, a possible change in actions. In other words, new conditions will not be added in theory.
In cases like this, you must follow OCP faithfully, create the base class and its extensions, or maintain the ifs? taking into consideration that new demands should not appear?
This is the current code that theoretically violates OCP.
Controller:
[HttpPost]
public ActionResult InserirOrcamento(OrcamentoViewModel orcamento, string acao)
{
if (acao == "consultaCPF")
{
var retornoConsultaCPF = OrcamentoService.consultaCPF(orcamento);
return Json(retornoConsultaCPF, JsonRequestBehavior.AllowGet);
}
if (acao == "listaTipoProduto")
{
var retornoTipoProduto = orcamento.lstTipoProduto = OrcamentoService.listaTipoProduto(orcamento);
return Json(retornoTipoProduto, JsonRequestBehavior.AllowGet);
}
}
Service
public class OrcamentoService
{
public static OrcamentoViewModel consultaCPF(OrcamentoViewModel orcamento)
{
using(ToqueEntities db = new ToqueEntities()){
var retorno = db.PESSOA_FISICA
.Where(x => x.PESF_CPF == orcamento.cpfPessoa)
// .Join(db.CONTATOS, x => x.PES_CDID, c => c.PES_CDID, (x,c) => new { x, c })
.Select(x => new OrcamentoViewModel
{
cpfPessoa = x.PESF_CPF,
nomeClienteOrcamento = x.PESF_NOME,
sobrenomeClienteOrcamento = x.PESF_SOBRENOME,
PES_CDID = x.PES_CDID
}).FirstOrDefault();
if (!retorno.Equals(null))
{
var lstContatos = db.CONTATOS.Where(x => x.PES_CDID == retorno.PES_CDID && x.CON_TIPOCONTATO != "EMAIL")
.Select(x => x.CON_CONTEUDO)
.ToList();
if (!lstContatos.Count().Equals(0))
{
foreach (var item in lstContatos)
{
retorno.Contatos += item + ";";
}
}
else
{
retorno.Contatos = "";
}
}
orcamento.nomeClienteOrcamento = retorno.nomeClienteOrcamento;
orcamento.sobrenomeClienteOrcamento = retorno.sobrenomeClienteOrcamento;
orcamento.cpfPessoa = retorno.cpfPessoa;
orcamento.Contatos = retorno.Contatos;
return orcamento;
}
}
thank you for commenting on this point of action.executarAcao(), had not seen that it has placed, it is not part of the code in question, is part of my attempt to implement the OCP, I will even edit.
– Gabriel Romão
I placed a string acao because they are actions executed via Ajax. For example, the CPF query is for when the user enters the CPF and takes the focus of the input, then the query is made and returns that person’s data in the respective inputs, such as the name and the surname. In other words, everything has to be in this Inserting View so that I can submit the form and give the proper processing. But I will try to break into 3 different actions, I appreciate the remark.
– Gabriel Romão
That’s not a view, that’s a action of a controller. Since you don’t know how to use MVC correctly you need to fix it, you shouldn’t put a action inside of another action, need to recast from that controller, even though I don’t sell the rest.
– Maniero
Yes, I just refactored this Controller, I separated the 2 actions that were inside it. Much more organized and better maintenance. Actually the problem of SRP was much more latent and could hinder maintenance in the future. I ended up expressing myself wrong in the placement above when I said "view", I was referring to view linked to that controller . Separating better, I saw that I could return only JSON and let JS handle the fields.
– Gabriel Romão
How you are using as Webapi, nor view has.
– Maniero
Actually no, it’s ASP.NET MVC same hahaha, just didn’t put the view and Javascript because I did not find pertinent to the question.
– Gabriel Romão