Registering an event participant, using Entity Framework and several associated models

Question

I have a method that is responsible for receiving an object and perform the information saved with Entity Framwork, using the Repository pattern. The issue is that this object is, basically, a DTO. The information contained in this object must be assigned to the models managed by the data layer, which is a bit confusing to me with respect to the way of making said assignments, considering that there are conditions for some of them.

public int RegisterParticipant(ParticipantDTO participant)
{
    try
    {
        EventoParticipante participante = new EventoParticipante();
        if(participant.Token != null)
        {
            var result = RegistrarPagoEnLinea(participant);
            participante.PagoEnLinea = true;
        }
        else
        {
            participante.PagoEnLinea = false;
            participante.TransaccionFecha = DateTime.Now;
            participante.TransaccionCodigo = participant.PaymentRefNumber;
        }
        participante.PersonaId = CreateNaturalPerson(participant).Id;
        participante.InstitucionId = short.Parse(CreateInstitution(participant).ToString());
        participante.EventoId = participant.EventId;
        participante.TipoParticipanteId = _attendeeTypeRepository.SingleOrDefault(x => x.Nombre == participant.TypeParticipant).Id;                
        participante.MontoPago = decimal.Parse(participant.Ammount);
        participante.Asistio = false;
        participante.FechaRegistro = DateTime.Now;
        participante.MontoPago = decimal.Parse(participant.Ammount) - 100;
        _eventAttendeeRepository.Add(participante);
        _eventAttendeeRepository.SaveChanges();
        Evento eventParticipant = _eventRepository.Get(participant.EventId);

        if (participant.CompanionName != null && !string.IsNullOrEmpty(participant.CompanionName))
        {
            Persona companion = new Persona();
            companion.Nombres = participant.CompanionName;
            companion.Apellidos = participant.CompanionLastName;
            companion.NumeroDocumento = participant.CompanionIdentity;
            companion.Ciudad = participant.AttendeeCity;
            companion.Activo = true;
            companion.UsuarioCreacion = "demo";
            companion.UsuarioModificacion = "demo";
            companion.FechaCreacion = DateTime.Now;
            companion.FechaModificacion = DateTime.Now;
            _personRepository.Add(companion);
            _personRepository.SaveChanges();

            EventoParticipante companionParticipant = new EventoParticipante();
            companionParticipant.PersonaId = companion.Id;
            companionParticipant.EventoParticipanteId = participante.Id;
            companionParticipant.EventoId = participante.EventoId;
            companionParticipant.MontoPago = decimal.Parse((100.00).ToString());
            companionParticipant.TipoParticipanteId = 4;
            companionParticipant.InstitucionId = participante.InstitucionId;
            companionParticipant.Asistio = false;
            companionParticipant.FechaRegistro = DateTime.Now;
            _eventAttendeeRepository.Add(companionParticipant);
            _eventAttendeeRepository.SaveChanges();
        }

        if (participant.InvoiceIdentity != null && !string.IsNullOrEmpty(participant.InvoiceIdentity))
        {
            CreateInvoice(participant, participante);
            MailHelper.SendMail(MailAction.EventRegisterSuccessWithInvoice, eventParticipant, participant, _countryRepository.Get(participant.AttendeeCountryId).Nombre, _countryRepository.Get((int)participant.InvoiceCountryId).Nombre);
        }
        else
            MailHelper.SendMail(MailAction.EventRegisterSuccess, eventParticipant, participant, _countryRepository.Get(participant.AttendeeCountryId).Nombre);
        return 0;
    }
    catch (Exception e)
    {
        return 1;
    }
}

As you can see, the method is excessively long. I understand that a method should be in charge of a specific function, so I know that it is wrong from that point.

To try to diminish its extension (and the possibility of errors in it) I have separated certain assignment tasks into methods within the same class, as in the case of CreateNaturalPerson (yes, the method was even longer). I thought to do the same with the IF where the values are assigned to Persona and EventoParticipante, but I think it would be to fall into the same. Is there any design pattern or advice for the above mentioned?

Answer 1

This method does too much:

• creates EventoParticipante
• creates Persona for the companion if necessary
• creates EventoParticipante for the companion if necessary
• CreateInvoice
• SendMail

Each of the first three should be a method or even a class of their own (especially considering each process seems to be fairly complicated), and then this RegisterParticipant method should perhaps be a class of its own to combine the individual parts as a process.

This one method is 60+ lines. That is already hard to keep an overview of, and it makes me wonder what other methods this class contains. Focus on making your methods simpler, apply SOLID,...

---

The mix of English and Spanish(?) as class names and method names and variable names is IMHO a bad idea.

---

Why do you do this: decimal.Parse((100.00).ToString());?

Ditto: short.Parse(CreateInstitution(participant).ToString());: why not simply have CreateInstitution return a short?

In the same vein: decimal.Parse(participant.Ammount);. Why isn't Ammount a decimal? (BTW this is spelled incorrectly, the correct word is "Amount".)

Such odd castings are all over your code, see also: (int)participant.InvoiceCountryId.

---

This will throw an exception if _attendeeTypeRepository cannot find anything:

_attendeeTypeRepository.SingleOrDefault(x => x.Nombre == participant.TypeParticipant).Id; 

Is that something you want?

---

Don't do this:

MailHelper.SendMail(MailAction.EventRegisterSuccessWithInvoice, 
   eventParticipant, 
   participant, 
   _countryRepository.Get(participant.AttendeeCountryId).Nombre, 
   _countryRepository.Get((int)participant.InvoiceCountryId).Nombre);

Even though it's only five parameters (which is already an amount that is bordering on my upper limit before I'd replace this with a custom class), at least two of these are the result of returns from other methods, and now you have a single line that is 250+ characters. This makes your code hard to read and follow.

Considering that you already pass participant, you could easily make a method that only accepts MailAction and participant, gets the other data itself, and then calls MailHelper.SendMail.

---

What does "4" mean in this context: companionParticipant.TipoParticipanteId = 4;? Avoid unexplained magic numbers: give them proper names or at least add a comment.