Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have this code but I think it is so messy and can me cleaned up a bit... But I can't because I don't know so much about C#...

The problem is that I have an if(servicio.Value == "PAS") because the query I want to execute vary depending on the Servicio.value, there can only be two "kind" of it, "PAS" and the rest, that can have ANY name, but not "PAS" because this one has an extra field in it's DB Table... I hope you can help me a bit, because this code isn't working correctly, it makes an infinite loop....

Thank you in advice

checkcenterDBDataContext dc = new checkcenterDBDataContext();
int intOkMas = Convert.ToInt32(txtUmbralOkMas.Text);
int intOkMenos = Convert.ToInt32(txtUmbralOkMenos.Text);
var days = GetDaysLikeMe(DateTime.Today).Take(50).Where(d => d.Date.Day < 28 && d.Date.Day > 1).Take(4);

XDocument xDoc = XDocument.Load("Settings.xml");
var Entidades =
    from ent in xDoc.Root.Elements("Entidad")
    select ent;
foreach (var entidad in Entidades)
{
    var Servicios =
        from serv in entidad.Element("Servicios").Elements("ServName")
        select serv;
    foreach (var servicio in Servicios)
    {
        if (servicio.Value == "PAS")
        {
            Umbrales newUmbral = new Umbrales();
            var Modulos =
                from mod in entidad.Element("Servicios").Elements("ModName")
                select mod;
            foreach (var modulo in Modulos)
            {
                for (int i = 0; i <= 2; i++)
                {
                    var valores =
                        from b in dc.PAS
                        where
                        b.Modulos.nombre == modulo.Value &&
                        b.Entidades.nombre == entidad.Element("Nombre").Value &&
                        b.peticion_id == i
                        && days.Contains(Convert.ToDateTime(b.fecha))
                        orderby b.id descending
                        select b;

                    var medias =
                        from b in valores
                        group b.valor by new { b.hora_id, b.dia_id, b.entidad_id, b.modulo_id, b.peticion_id } into hg
                        orderby hg.Key.hora_id descending
                        select new
                        {
                            Hora = hg.Key.hora_id,
                            Dia = hg.Key.dia_id,
                            Entidad = hg.Key.entidad_id,
                            Modulo = hg.Key.modulo_id,
                            Peticion = hg.Key.peticion_id,
                            Maximo = Math.Round(System.Convert.ToDouble(hg.Average() + ((hg.Average() * intOkMas) / 100)), 3, MidpointRounding.AwayFromZero),
                            Minimo = Math.Round(System.Convert.ToDouble(hg.Average() - ((hg.Average() * intOkMenos) / 100)), 3, MidpointRounding.AwayFromZero)
                        };

                    foreach (var med in medias)
                    {
                        var update_medias =
                            (from um in dc.Umbrales
                             where
                             um.hora_id == med.Hora
                             && um.dia_id == med.Dia
                             && um.entidad_id == med.Entidad
                             && um.servicio_id == 3
                             && um.peticion_id == med.Peticion
                             && um.modulo_id == med.Modulo
                             select um).ToList();
                        if (update_medias.Any())
                        {
                            foreach (var ume in update_medias)
                            {
                                ume.maximo = Convert.ToDecimal(med.Maximo);
                                ume.minimo = Convert.ToDecimal(med.Minimo);
                                dc.SubmitChanges();
                            }
                        }
                        else
                        {
                            newUmbral.hora_id = med.Hora;
                            newUmbral.dia_id = med.Dia;
                            newUmbral.entidad_id = med.Entidad;
                            newUmbral.servicio_id = 3;
                            newUmbral.peticion_id = med.Peticion;
                            newUmbral.modulo_id = med.Modulo;
                            newUmbral.maximo = Convert.ToDecimal(med.Maximo);
                            newUmbral.minimo = Convert.ToDecimal(med.Minimo);
                            dc.Umbrales.InsertOnSubmit(newUmbral);
                            dc.SubmitChanges();
                        }
                    }

                }
            }
        }
        else
        {
            Umbrales newUmbral = new Umbrales();
            for (int i = 0; i <= 2; i++)
            {
                var valores =
                    from b in dc.Banksphere
                    where
                    b.Servicios.nombre == servicio.Value &&
                    b.Entidades.nombre == entidad.Element("Nombre").Value &&
                    b.peticion_id == i
                    && days.Contains(Convert.ToDateTime(b.fecha))
                    orderby b.id descending
                    select b;

                var medias =
                    from b in valores
                    group b.valor by new { b.hora_id, b.dia_id, b.entidad_id, b.servicio_id, b.peticion_id } into hg
                    orderby hg.Key.hora_id descending
                    select new
                    {
                        Hora = hg.Key.hora_id,
                        Dia = hg.Key.dia_id,
                        Entidad = hg.Key.entidad_id,
                        Servicio = hg.Key.servicio_id,
                        Peticion = hg.Key.peticion_id,
                        Maximo = Math.Round(System.Convert.ToDouble(hg.Average() + ((hg.Average() * intOkMas) / 100)), 3, MidpointRounding.AwayFromZero),
                        Minimo = Math.Round(System.Convert.ToDouble(hg.Average() - ((hg.Average() * intOkMenos) / 100)), 3, MidpointRounding.AwayFromZero)
                    };

                foreach (var med in medias)
                {
                    var update_medias =
                        (from um in dc.Umbrales
                         where
                         um.hora_id == med.Hora
                         && um.dia_id == med.Dia
                         && um.entidad_id == med.Entidad
                         && um.servicio_id == med.Servicio
                         && um.peticion_id == med.Peticion
                         select um).ToList();
                    if (update_medias.Any())
                    {
                        foreach (var ume in update_medias)
                        {
                            ume.maximo = Convert.ToDecimal(med.Maximo);
                            ume.minimo = Convert.ToDecimal(med.Minimo);
                            dc.SubmitChanges();
                        }
                    }
                    else
                    {
                        newUmbral.hora_id = med.Hora;
                        newUmbral.dia_id = med.Dia;
                        newUmbral.entidad_id = med.Entidad;
                        newUmbral.servicio_id = med.Servicio;
                        newUmbral.peticion_id = med.Peticion;
                        newUmbral.maximo = Convert.ToDecimal(med.Maximo);
                        newUmbral.minimo = Convert.ToDecimal(med.Minimo);
                        dc.Umbrales.InsertOnSubmit(newUmbral);
                        dc.SubmitChanges();
                    }
                }

            }
        }
    }
}
share|improve this question

closed as off topic by Jeff Vanzella, codesparkle, Trevor Pilley, James Khoury, seand Nov 20 '12 at 17:17

Questions on Code Review Stack Exchange are expected to relate to code review request within the scope defined by the community. Consider editing the question or leaving comments for improvement if you believe the question can be reworded to fit within the scope. Read more about reopening questions here.If this question can be reworded to fit the rules in the help center, please edit the question.

2  
For a start: Less indentation, more methods. –  avip Nov 14 '12 at 17:37

1 Answer 1

up vote 1 down vote accepted

A few points to note first i see that you are coding in spanish. I'm going to do the example using my coding standard since i don't agree with adding int to the variable name since it only confuses the variable name more. Therefore things that i would change:

  1. Variable Names
  2. Use var for obvious variables
  3. Create a Method for each one of the foreachs Similar to a composed method approach(http://www.ibm.com/developerworks/java/library/j-eaed4/index.html)
  4. If you iterating over only a selection of the collection combine the if and on the foreach

Did a few examples left the rest to be done by op.

     //Might want to add parameters here instead of hardcoded values
     private Days GetDays()
     {
        return GetDaysLikeMe(DateTime.Today).Take(50).Where(d => d.Date.Day < 28 && d.Date.Day > 1).Take(4);
     }


     checkcenterDBDataContext dc = new checkcenterDBDataContext();
     var OkMas = Convert.ToInt32(txtUmbralOkMas.Text);
     var OkMenos = Convert.ToInt32(txtUmbralOkMenos.Text);
     var days = GetDays();
     constant string Filename ="Settings.xml"
     XDocument xDoc = XDocument.Load(Filename);
     var Entidades =
         from ent in xDoc.Root.Elements("Entidad")
         select ent;
     foreach (var entidad in Entidades)
     {
        ProcessEntity(entidad);
     }
     //Not sure the type
      private void ProcessEntity(object entidad)
       {
        var Servicios =
             from serv in entidad.Element("Servicios").Elements("ServName")
             select serv;
         //point number 4
         foreach (var servicio in Servicios.Select(servicio.Value == "PAS"))
         {
         //Split into a separate method
        Umbrales newUmbral = new Umbrales();
        var Modulos =
            from mod in entidad.Element("Servicios").Elements("ModName")
            select mod;
        foreach (var modulo in Modulos)
        {
            for (int i = 0; i <= 2; i++)
            {
                var valores =
                    from b in dc.PAS
                    where
                    b.Modulos.nombre == modulo.Value &&
                    b.Entidades.nombre == entidad.Element("Nombre").Value &&
                    b.peticion_id == i
                    && days.Contains(Convert.ToDateTime(b.fecha))
                    orderby b.id descending
                    select b;

                var medias =
                    from b in valores
                    group b.valor by new { b.hora_id, b.dia_id, b.entidad_id, b.modulo_id, b.peticion_id } into hg
                    orderby hg.Key.hora_id descending
                    select new
                    {
                        Hora = hg.Key.hora_id,
                        Dia = hg.Key.dia_id,
                        Entidad = hg.Key.entidad_id,
                        Modulo = hg.Key.modulo_id,
                        Peticion = hg.Key.peticion_id,
                        Maximo = Math.Round(System.Convert.ToDouble(hg.Average() + ((hg.Average() * intOkMas) / 100)), 3, MidpointRounding.AwayFromZero),
                        Minimo = Math.Round(System.Convert.ToDouble(hg.Average() - ((hg.Average() * intOkMenos) / 100)), 3, MidpointRounding.AwayFromZero)
                    };

                foreach (var med in medias)
                {
                    var update_medias =
                        (from um in dc.Umbrales
                         where
                         um.hora_id == med.Hora
                         && um.dia_id == med.Dia
                         && um.entidad_id == med.Entidad
                         && um.servicio_id == 3
                         && um.peticion_id == med.Peticion
                         && um.modulo_id == med.Modulo
                         select um).ToList();
                    if (update_medias.Any())
                    {
                        foreach (var ume in update_medias)
                        {
                            ume.maximo = Convert.ToDecimal(med.Maximo);
                            ume.minimo = Convert.ToDecimal(med.Minimo);
                            dc.SubmitChanges();
                        }
                    }
                    else
                    {
                        newUmbral.hora_id = med.Hora;
                        newUmbral.dia_id = med.Dia;
                        newUmbral.entidad_id = med.Entidad;
                        newUmbral.servicio_id = 3;
                        newUmbral.peticion_id = med.Peticion;
                        newUmbral.modulo_id = med.Modulo;
                        newUmbral.maximo = Convert.ToDecimal(med.Maximo);
                        newUmbral.minimo = Convert.ToDecimal(med.Minimo);
                        dc.Umbrales.InsertOnSubmit(newUmbral);
                        dc.SubmitChanges();
                    }
                }

            }
        }
    }
    else
    {
        Umbrales newUmbral = new Umbrales();
        for (int i = 0; i <= 2; i++)
        {
            var valores =
                from b in dc.Banksphere
                where
                b.Servicios.nombre == servicio.Value &&
                b.Entidades.nombre == entidad.Element("Nombre").Value &&
                b.peticion_id == i
                && days.Contains(Convert.ToDateTime(b.fecha))
                orderby b.id descending
                select b;

            var medias =
                from b in valores
                group b.valor by new { b.hora_id, b.dia_id, b.entidad_id, b.servicio_id, b.peticion_id } into hg
                orderby hg.Key.hora_id descending
                select new
                {
                    Hora = hg.Key.hora_id,
                    Dia = hg.Key.dia_id,
                    Entidad = hg.Key.entidad_id,
                    Servicio = hg.Key.servicio_id,
                    Peticion = hg.Key.peticion_id,
                    Maximo = Math.Round(System.Convert.ToDouble(hg.Average() + ((hg.Average() * intOkMas) / 100)), 3, MidpointRounding.AwayFromZero),
                    Minimo = Math.Round(System.Convert.ToDouble(hg.Average() - ((hg.Average() * intOkMenos) / 100)), 3, MidpointRounding.AwayFromZero)
                };

            foreach (var med in medias)
            {
                var update_medias =
                    (from um in dc.Umbrales
                     where
                     um.hora_id == med.Hora
                     && um.dia_id == med.Dia
                     && um.entidad_id == med.Entidad
                     && um.servicio_id == med.Servicio
                     && um.peticion_id == med.Peticion
                     select um).ToList();
                if (update_medias.Any())
                {
                    foreach (var ume in update_medias)
                    {
                        ume.maximo = Convert.ToDecimal(med.Maximo);
                        ume.minimo = Convert.ToDecimal(med.Minimo);
                        dc.SubmitChanges();
                    }
                }
                else
                {
                    newUmbral.hora_id = med.Hora;
                    newUmbral.dia_id = med.Dia;
                    newUmbral.entidad_id = med.Entidad;
                    newUmbral.servicio_id = med.Servicio;
                    newUmbral.peticion_id = med.Peticion;
                    newUmbral.maximo = Convert.ToDecimal(med.Maximo);
                    newUmbral.minimo = Convert.ToDecimal(med.Minimo);
                    dc.Umbrales.InsertOnSubmit(newUmbral);
                    dc.SubmitChanges();
                }
            }

        }
    }
}
share|improve this answer

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