Is it wrong to put this much code into a button click event?

Question

It is wrong to put this amount of code in an event of click button?

private void CPeBTNSalvar_Click(object sender, EventArgs e)
    {
        try
        {
            if (CPeTBCodigo.Text == "")
            {
                throw new ExcecaoModificada("Determine um codigo para o pedido!");
            }

            int codigo = int.Parse(CPeTBCodigo.Text);
            int ano = CpeData.Value.Year;
            int mes = CpeData.Value.Month;
            int dia = CpeData.Value.Day;
            bool semPedido = true;

            Pedido p = new Pedido(codigo, ano, mes, dia);

            foreach (DataGridViewRow x in dG_CPeListarProdutos.Rows)
            {
                if (Convert.ToInt32(x.Cells[4].Value) > 0)
                {
                    ItemPedido item = new ItemPedido(FormularioPrincipal.lProduto[x.Index], Convert.ToInt32(x.Cells[4].Value), Convert.ToDouble(x.Cells[3].Value));
                    p.itens.Add(item);
                    semPedido = false;
                    x.Cells[3].Value = 0;

                    x.Cells[4].Value = 0;
                }
                else
                {
                    x.Cells[3].Value = 0;
                }
            }
            if (semPedido == true)
            {
                throw new ExcecaoModificada("Escolha algum produto!");
            }
            else
            {
                CPeLabelAlerta.Visible = true;
                CPeLabelAlerta.Text = "Novo pedido adicionado!";
                CPeLabelAlerta.ForeColor = Color.DarkSlateGray;
                FormularioPrincipal.lPedido.Add(p);
            }
        }
        catch (ExcecaoModificada erro)
        {
            CPeLabelAlerta.Visible = true;
            CPeLabelAlerta.Text = erro.Message;
            CPeLabelAlerta.ForeColor = Color.Red;
        }
        catch (Exception)
        {
            CPeLabelAlerta.Visible = true;
            CPeLabelAlerta.Text = "Coloque apenas numeros em código!";
        }
    }

If you think it’s not wrong, quote something you wouldn’t do like I did.

Answer 1

It should only have the most obvious code to execute when the button is clicked.

A part of the code does not seem to have to do with the button itself, and should therefore be delegated to other methods, such as validation and preparation of other parts of the screen. It also depends a lot on the rest of the code to evaluate. In general they will be private or even belonging to another class (in the case of validation itself that is business rule and not screen rule).

There actually seems to be far more important errors in this code, like the Pedido which is pretty weird, the form of data conversion is prone to errors, the use of exceptions is wrong in every sense. It’s just not easy to tidy up without knowing the whole problem. The problem seems to be structural, and so we could not fix only this detail, would have to fix the entire application.

Answer 2

There is a lot of redundancy in your code. The event CPeBTNSalvar_Click will request the registration of something in the database or file? It seems that it is only searching the data of DataGridView and adding to your class, and as @Maniero said, it should only have the necessary code. Make the names clearer and readable too, only use prefix if you have any worthwhile benefits.

Validation of numerical data

In your code just one method to validate

if (int.TryParse(CPeTBCodigo.Text, out var resultado)){ ... }

No need to validate more than once as CPeTBCodigo.Text == "" and int.Parse(CPeTBCodigo.Text);, as this in your code.

Exceptions

You can learn the basics of how use exceptions here, always try to avoid them. Unnecessary exceptions can get in the way of tracking some error or impair the performance of the application. And in your case you use exceptions to treat data that comes from the user, there is no need to create exceptions to this.

Business rules

However, if you want you can use the Dataannotations to set the meta data for the control of the data kept in the object. Next you a ValidationContext to describe the context of the validation and ValidationResult to get the validation result and error messages.

Small example of illustration

The example below illustrates how to use the resources cited above, see:

using System.ComponentModel.DataAnnotations; // Tem que adicionar esta referencia ao seu projeto.

public class Produto
{
    public long Id { get; set; }
    
    [Required(ErrorMessage = "Este campo é obrigatório.")]
    public string Descricao { get; set; }
    
    [Range(1.0, Double.MaxValue, ErrorMessage = "Oops... o preço não pode ser 0 (zero) ou menor.")]
    public decimal Preco { get; set; }
    
    
    public void Validate()
    {
        Produto produto = this;
        ValidationContext context = new ValidationContext(produto, null, null);
        IList<ValidationResult> erros = new List<ValidationResult>();
        
        if (!Validator.TryValidateObject(cliente, context, erros, true))
        {
            foreach (ValidationResult resultado in erros)
            {
                MinhaListaDeErros.Add(resultado.ErrorMessage);
            }
        }    
        
        if (!MinhaOutraValidacao()) MinhaListaDeErros.Add("Minha outra mensagem de erro.");
    }    
}

The above example is to show a path, there is much that needs to be improved, so be careful, and consider the context and necessity of its application.

Learn more about Dataannotations.

More about field validation.