Конкурс “Смеяться или плакать?”

Преамбула

Однажды я пришёл на работу чуть раньше обычного и, дабы не терять времени, решил просмотреть исходники в текущем проекте. В результате родилась заметка о довольно интересном куске кода. Прочитав ее, мой друг нашел в своем проекте похожие "творения". Вот так родилась идея провести конкурс на самый плохой код.

Правила

Правила конкурса достаточно просты: нужно найти в своём текущем проекте участок плохого кода (можно воспользоваться системой контроля версий, если Вы уже провели рефакторинг), прислать мне на электронную почту письмо, указав язык программирования, код и, по возможности, способ его рефакторинга. Вы так же можете оставлять примеры кода прямо в комментариях (на забудьте заключить куски кода в блок <code lang="language">some code</code>). Все примеры, в которых действительно есть “запашок” (code smell), будут публиковаться в этой заметке в разделе Конкурсные работы (если вы не хотите называть свое имя, укажите это в письме).

Например,

Язык: Java

Код:

if (value) {
   for (int i = 0; i < list.size(); i++) {
     ((Column)list.get(i)).visible = false;
   }
} else {
    for (int i = 0; i < list.size(); i++) {
       ((Column)list.get(i)).visible = true;
    }
}

Рефакторинг:

for (int i = 0; i < list.size(); i++) {
 ((Column)list.get(i)).visible = !value;
}

Еще один пример можно увидеть в моей предыдущей заметке.

Примеры кода будут накапливаться в течение трех недель (19 Июля — 9 Августа). Если вы не можете (или не хотите) публиковать свой код,- оставляйте свои комментарии по поводу чужих творений, все они будут учитываться при выборе финалистов. В конце третьей недели из всех участвующий примеров кода выбираются пять финалистов, за которые смогут голосовать все желающие в течение одной недели (10 Августа — 16 Августа).

Итак, 3 недели на сбор кода, 1 неделя на голосование и что в итоге? Итак, самый кошмарный, невообразимо жуткий и пугающий кусок кода получает…

Призы

Какой же конкурс без призов? Итак, победитель получает приз в виде оригинала книги Макконнелла "Code Complete", либо ее русский перевод "Совершенный код". После того, как победитель будет определен(17 Августа), я с ним свяжусь по email для уточнения адреса доставки, и закажу с Amazon либо books.ru книгу прямо ему домой.

Зачем это нужно?

Каждый день мы встречаемся с примерами кода, мягко говоря, устрашающими. Здесь и классический паттерн copy-paste, и гениальные по своей сути десятиэтажные условные операторы (разобраться в которых не под силу даже их автору), и просто забавные огрехи рефакторинга, при которых сущность Person физически располагается в таблице products. Каждый в своей практике сталкивался и будет сталкиваться с подобным. Так выскажете же все, что накипело, покажите людям, как делать не надо, и выиграйте приз!

Конкурсные работы

Автор: COTOHA
Язык: C#

Код:

if(Read)
 {
      for (int index = 0; index < messageInfo.Count; index++)
     {
        messageInfo[index].Status = false;
        messageInfoDataSource.Update(messageInfo[index]);
     }
 }
 else
 {
    for (int index = 0; index < messageInfo.Count; index++)
    {
       messageInfo[index].Status = true;
       messageInfoDataSource.Update(messageInfo[index]);
    }
  }

Рефакторинг:

for (int index = 0; index < messageInfo.Count; index++)
 {
    messageInfo[index].Status = !Read;
    messageInfoDataSource.Update(messageInfo[index]);
 }

This entry was posted on Thursday, July 19th, 2007 at 0:04 and is filed under development. You can follow any responses to this entry through the RSS 2.0 feed. You can leave a response, or trackback from your own site.

70 Responses to “Конкурс “Смеяться или плакать?””

  • Dmytro Shteflyuk says:

    Let’s get started. For the beginning here is the simple copy/paste pattern:
    Language: C#

    if (!ValidateString(text))
    {
        divNormal.Visible = false;
        divError.Visible = true;
        divForm.Visible = true;
    }
    else
    {
        divNormal.Visible = true;
        divError.Visible = false;
        divForm.Visible = false;
    }

    Refactoring:

    bool isValid = ValidateString(text);
    divNormal.Visible = isValid;
    divError.Visible = !isValid;
    divForm.Visible = !isValid;

    19.07.2007 at 0:13

  • Dmytro Shteflyuk says:

    Event better exception handling
    Language: C#

    if (dv.Count > 0)
    {
        try
        {
            if (Convert.ToString(dv.Table.Rows[0].ItemArray[dv.Table.Columns.IndexOf("ME_Status")]) == "A")
            {
                MemberID = Convert.ToInt32(dv.Table.Rows[0].ItemArray[dv.Table.Columns.IndexOf(MemberDB.FieldName.Id)]);
                if (!IsPostBack)
                {
                    try { ZipWork.Text = (string)dv.Table.Rows[0].ItemArray[dv.Table.Columns.IndexOf(MemberDB.FieldName.ZipWork)];} catch { }
                    try { if ((bool)dv.Table.Rows[0].ItemArray[dv.Table.Columns.IndexOf(MemberDB.FieldName.Smoking)]) SmokingY.Checked = true; else SmokingN.Checked = true;} catch { }
                    try { SexOrient.SelectedIndex = SexOrient.Items.IndexOf(SexOrient.Items.FindByValue((string)dv.Table.Rows[0].ItemArray[dv.Table.Columns.IndexOf(MemberDB.FieldName.SexSelect)]));} catch { }
                    try { Marital.SelectedIndex = Marital.Items.IndexOf(Marital.Items.FindByValue((string)dv.Table.Rows[0].ItemArray[dv.Table.Columns.IndexOf(MemberDB.FieldName.Marital)])); } catch { }
                }
            }
            else
            {
                Response.Redirect("~/Member.aspx", false);
            }
        }
        catch
        {
            Response.Redirect("~/Member.aspx", false);
        }
    }
    else
    {
        Response.Redirect("~/Member.aspx", false);
    }

    Refactoring:
    Kill the author of this code.

    19.07.2007 at 0:53

  • COTOHA says:

    well. actually my example was for C#.

    19.07.2007 at 10:37

  • erka says:

    Yep. You are right. I forgot. I have updated your example. C# is really popular :)

    19.07.2007 at 10:57

  • Dmytro Shteflyuk says:

    COTOHA: Еще есть? :-) Неужели только я работаю с говняным кодом по жизни?

    19.07.2007 at 11:11

  • COTOHA says:

    Критик, это что-ли твой ацкий пример претендующий на 1е место? могу сказать, что это слабо. очень слабо…

    19.07.2007 at 11:43

  • Dmytro Shteflyuk says:

    COTOHA: не, ты что. Нельзя же сразу аццкие отжиги выкладывать. Я сначала разогрею :-) Так ты будешь что-то постить, или снова мне? Отвечу сам себе:

    Разворачиваем цикл
    Language: C#

    private string StatItemsClientId()
    {
        StringBuilder str = new StringBuilder();
        str.Append("\"");
        //Id of Message stat
        str.Append(((c_StatItem)this.FindControl("StatItem1")).getLabelId(1));
        str.Append("\",\"");
        str.Append(((c_StatItem)this.FindControl("StatItem1")).getLabelId(2));
        str.Append("\",\"");
        //Id of FileQuota stat1
        str.Append(((c_StatItem)this.FindControl("StatItem2")).getLabelId(1));
        str.Append("\",\"");
        str.Append(((c_StatItem)this.FindControl("StatItem2")).getLabelId(2));
        str.Append("\",\"");
        //Id of Reply stat
        str.Append(((c_StatItem)this.FindControl("StatItem3")).getLabelId(null));
        str.Append("\",\"");
        //id of Offer stat
        str.Append(((c_StatItem)this.FindControl("StatItem4")).getLabelId(null));
        str.Append("\",\"");
        //Id of Autentification stat
        str.Append(((c_StatItem)this.FindControl("StatItem5")).getLabelId(null));
        str.Append("\",\"");
        //Id of Stat Body
        str.Append(this.FindControl("StatBody").ClientID);
        str.Append("\",\"");
        //Id of Stat Error panel
        str.Append(this.FindControl("StatError").ClientID);
        str.Append("\"");
        return str.ToString();
    }

    19.07.2007 at 13:09

  • Dmytro Shteflyuk says:

    Если так и дальше пойдет, я выиграю только потому, что больше никто не участвует :-)

    19.07.2007 at 13:15

  • Dmytro Shteflyuk says:

    Erka, давай хоть ты напости. Я тебе лично подарю эту книжку, если найдешь код круче всего, что было и будет предложено.

    19.07.2007 at 13:31

  • COTOHA says:

    ща отожгу…

    19.07.2007 at 14:14

  • COTOHA says:

    ладно, чуваки и чувихи - рыдайте! ибо грядёт победитель всех победителей и гроза всех прочих недопретендентов… РЕАЛЬНО АЦЦКИЙ ОТЖИГ:

    Language: C#

    private enum MessageBoxFields
     {
     CheckBox = 0,
     Sender = 1,
     SenderAvatar = 2,
     Recipient = 3,
     RecipientAvatar = 4,
     Subject = 5,
     FriendRequest = 6,
     PendingRequest = 7,
     Date = 8,
     Time = 9,
     Read = 10,
     MessageID = 11
    }

    private void MessageBoxVisibleFields(List<int> ShowFields)
    // сюда мы передаем список колонок, которые надо показать в датагриде
    {
    List<int> AllFields = new List<int>();
    // заметте, что ТУТ мы не верим в неявное приведение типов
    // поэтому мы юзаем Convert.ToInt32
      AllFields.Add(Convert.ToInt32(MessageBoxFields.CheckBox));
      AllFields.Add(Convert.ToInt32(MessageBoxFields.Sender));
      AllFields.Add(Convert.ToInt32(MessageBoxFields.SenderAvatar));
      AllFields.Add(Convert.ToInt32(MessageBoxFields.Recipient));
      AllFields.Add(Convert.ToInt32(MessageBoxFields.RecipientAvatar));
      AllFields.Add(Convert.ToInt32(MessageBoxFields.Subject));
      AllFields.Add(Convert.ToInt32(MessageBoxFields.FriendRequest));
      AllFields.Add(Convert.ToInt32(MessageBoxFields.PendingRequest));
      AllFields.Add(Convert.ToInt32(MessageBoxFields.Date));
      AllFields.Add(Convert.ToInt32(MessageBoxFields.Time));
      AllFields.Add(Convert.ToInt32(MessageBoxFields.Read));
      AllFields.Add(Convert.ToInt32(MessageBoxFields.MessageID));
    // мы только что сформировали список всех колонок ;)

      bool flag = false;
      List<int> ShowF = new List<int>(); // здесь будут лежать колонки, которые мы покажем.
                                               // да-да те, которые нам передали как параметр 8-/
      List<int> HideF = new List<int>(); // а тут те, которые мы спрячем

    // глубина этого алгоритма непостижима простому смертному, но я же СОТОНА!
    // поэтому я вам всё как есть расскажу

    // как в любой задаче по дискретной математике, бежим по матрице
      for (int i = 0; i < AllFields.Count; i++) // это столбцы :)
      {
         for (int j = 0; j < ShowFields.Count; j++) // а тут колонки.
          {
             if (AllFields[i] == ShowFields[j]) // а тут у нас пересечение.
             {
                ShowF.Add(ShowFields[j]);
                j = ShowFields.Count; // WTF!?!
                flag = true;          // ДА ЭТО ЖЕ BREAK!!! мама-мия!!! 
             }
              // только за эту реализацию break я требую и оригиналы и переводы 
              // code complete, the beautiful code, refactoring ну и prefactoring до кучи
          }
          if (!flag) // ну если пересечение не случилось, то придётся поле спрятать
          {
             HideF.Add(AllFields[i]);
          }
          flag = false; // ну да. а вдруг таки случалось пересечение
       }

    // ну дальше просто детский лепет
    // надо просто показать поля, которые надо показать
       for (int i = 0; i < ShowF.Count; i++)
       {
          // помните мы там замечали, что мы не верим в неявное приведение типов?
          // так вот мы уверовали! алилуйя!!!
          MessagesGridView.Columns[ShowF[i]].Visible = true;
       }
      // и спрятать те, которые надо спрятать
       for (int i = 0; i < HideF.Count; i++)
       {
         MessagesGridView.Columns[HideF[i]].Visible = false;
       }
    }

    ну что, ламера? прорыдались?!? у вас нет даже и близко похожего :)

    рефакторить будем? лучше не надо, т.к. придётся показать методы формирования List ShowFields для каждого кейса использоваия гридвьюва, а этого я могу не выдержать…

    19.07.2007 at 14:45

  • COTOHA says:

    to Разворачиваем цикл

    и чё? это что за петросянщина? нифига не смешно…

    19.07.2007 at 14:49

  • Dmytro Shteflyuk says:

    Да, чувак, ты зажег. Боюсь, в текущем проекте такого не найти, придется поднимать старый… Но умывать руки рано :-) Я буду драться до последнего, у меня еще в запасе есть немного пороха.

    19.07.2007 at 15:08

  • COTOHA says:

    ага. тут главное обратить внимание на элегантность копирования массива ShowFields в ShowF. ну и вообще на безудержную фантазию автора.

    ну и это не я зажёг - я тушить пытаюсь.

    19.07.2007 at 15:10

  • AntonShevchuk says:

    Классику построения дерева категорий с произвольной вложенностью используя рекурсию с обращением к БД на каждой итерации показать???

    19.07.2007 at 15:56

  • ikar says:

    Language: Java

    public static String[][] getGMailAdreses(String login,
                                       String domain, String password){
            if(domain.equals(null)) domain = "gmail.ru";
    }

    Автор пытается защититься от вариантов передачи параметра domain=null, но при этом даже не тестил свою защиту

    19.07.2007 at 15:57

  • erka says:

    Don’t trust a variable “live”
    Language: Java
    Code:

    public class  RunnableImpl implements Runnable{
     protected boolean isTerminating = false;
     protected boolean live = false;

     public void stop() {
      isTerminating = true;
     }
     
     public boolean isLive() {
      return (live && !isTerminating);
     }
     public void run() {
      isTerminating = false;
      live = true;
      execute();
      live = false;
      isTerminating = false;
     }
    }

    Refactoring:

    public class  RunnableImpl implements Runnable{
     protected boolean isTerminating = false;
     protected boolean live = false;

     public void stop() {
      isTerminating = true;
     }
     
     public boolean isLive() {
      return live;
     }
     public void run() {
      live = true;
      try {
        execute();
      } finally {
        live = false;
      }
     }
    }

    19.07.2007 at 16:03

  • ikar says:

    Language: Java

    try{
       temp = new ArrayList();
       int i = 0;
       while(true){
            if((pos = ans[i].indexOf("href=")) == -1) {
                 i++;
            }else{
                str = ans[i].substring(pos + 5, ans[i].indexOf(">", pos));

                if((pos = str.indexOf("\"")) != -1){
                  str = str.substring(pos+1, str.indexOf("\"", pos+1));
                }

                temp.add(str);
                i++;
           }
       }
    }catch(IndexOutOfBoundsException ex){}

    Обалденный способ прохода по элементам массива

    19.07.2007 at 16:03

  • erka says:

    Ikar: а чего оно все в одну строчку бросает?

    Моя ошибка, но при модерации я исправляю. Для тега code нужно еще добавить атрибут lang="language" (например, lang="java", lang="c#"). Спасибо за вопрос.

    19.07.2007 at 16:18

  • erka says:

    Just new update from subversion

    Classic CPD
    Language: Java
    Code:

    if (attrsMap.containsKey("db")) {
      operationBean.setDb(attrsMap.get("db").toString());
    } else {
      throw new Exception("Can't find db attr!");
    }

    if (attrsMap.containsKey("name")) {
      operationBean.setName(attrsMap.get("name").toString());
    } else {
      throw new Exception("Can't find name attr!");
    }
    if (attrsMap.containsKey("order")) {
      operationBean.setOrder(attrsMap.get("order").toString());
    } else {
      throw new Exception("Can't find order attr!");
    }

    if (attrsMap.containsKey("orderedparams")) {
      operationBean.setOrderedparams(attrsMap.get("orderedparams").toString());
    } else {
      throw new Exception("Can't find orderedparams attr!");
    }

    if (attrsMap.containsKey("sql")) {
      operationBean.setSql(attrsMap.get("sql").toString());
    } else {
      throw new Exception("Can't find sql attr!");
    }

    if (attrsMap.containsKey("table")) {
      operationBean.setTable(attrsMap.get("table").toString());
    } else {
      throw new Exception("Can't find table attr!");
    }

    if (attrsMap.containsKey("type")) {
      operationBean.setType(attrsMap.get("type").toString());
    } else {
      throw new Exception("Can't find type attr!");
    }

    Refactoring:

    operationBean.setDb(getAttributeValue(attrsMap, "db"));
    operationBean.setName(getAttributeValue(attrsMap, "name"));
    // ....
    operationBean.setType(getAttributeValue(attrsMap, "type"));

    private String getAttributeValue(HashMap attrsMap, String key)
       throws Exception {
     if (attrsMap.containsKey(key)) {
        return (String)attrsMap.get(key);
      } else {
        throw new Exception("Can't find " + key+ " attr!");
      }
    }

    19.07.2007 at 17:17

  • koba says:

    Стоящих аргументов из текущего проекта привести не могу, но смог вспомнить что где-то попадалось такое:

    Boolean result = executeCommand(params);
    if (result.toString().length() == 5) {
        System.out.println("command failed");
    } else if (result.toString().length() == 4) {
        System.out.println("command successful");
    } else {
        throw new Exception("something wrong");
    }

    19.07.2007 at 20:30

  • Dmytro Shteflyuk says:

    Ааааааааааа, народ. Вы разрываете мне моск!

    Правильная реализация календаря
    Language: C#

    if (ActualDate.Date == new DateTime(ActualDate.Year, ActualDate.Month, 1).AddDays(-1)) ActualDate = ActualDate.AddMonths(interval - 1);
    ActualDate = ActualDate.AddDays(1);
    while (!((RecurringMonthlyEveryDay1.Checked & ActualDate.Day == 1) |
        (RecurringMonthlyEveryDay1.Checked & ActualDate.Day == 1) |
        (RecurringMonthlyEveryDay2.Checked & ActualDate.Day == 2) |
        (RecurringMonthlyEveryDay3.Checked & ActualDate.Day == 3) |
        (RecurringMonthlyEveryDay4.Checked & ActualDate.Day == 4) |
        (RecurringMonthlyEveryDay5.Checked & ActualDate.Day == 5) |
        (RecurringMonthlyEveryDay6.Checked & ActualDate.Day == 6) |
        (RecurringMonthlyEveryDay7.Checked & ActualDate.Day == 7) |
        (RecurringMonthlyEveryDay8.Checked & ActualDate.Day == 8) |
        (RecurringMonthlyEveryDay9.Checked & ActualDate.Day == 9) |
        (RecurringMonthlyEveryDay10.Checked & ActualDate.Day == 10) |
        (RecurringMonthlyEveryDay11.Checked & ActualDate.Day == 11) |
        (RecurringMonthlyEveryDay12.Checked & ActualDate.Day == 12) |
        (RecurringMonthlyEveryDay13.Checked & ActualDate.Day == 13) |
        (RecurringMonthlyEveryDay14.Checked & ActualDate.Day == 14) |
        (RecurringMonthlyEveryDay15.Checked & ActualDate.Day == 15) |
        (RecurringMonthlyEveryDay16.Checked & ActualDate.Day == 16) |
        (RecurringMonthlyEveryDay17.Checked & ActualDate.Day == 17) |
        (RecurringMonthlyEveryDay18.Checked & ActualDate.Day == 18) |
        (RecurringMonthlyEveryDay19.Checked & ActualDate.Day == 19) |
        (RecurringMonthlyEveryDay20.Checked & ActualDate.Day == 20) |
        (RecurringMonthlyEveryDay21.Checked & ActualDate.Day == 21) |
        (RecurringMonthlyEveryDay22.Checked & ActualDate.Day == 22) |
        (RecurringMonthlyEveryDay23.Checked & ActualDate.Day == 23) |
        (RecurringMonthlyEveryDay24.Checked & ActualDate.Day == 24) |
        (RecurringMonthlyEveryDay25.Checked & ActualDate.Day == 25) |
        (RecurringMonthlyEveryDay26.Checked & ActualDate.Day == 26) |
        (RecurringMonthlyEveryDay27.Checked & ActualDate.Day == 27) |
        (RecurringMonthlyEveryDay28.Checked & ActualDate.Day == 28) |
        (RecurringMonthlyEveryDay30.Checked & ActualDate.Day == 30) |
        (RecurringMonthlyEveryDay31.Checked & ActualDate.Day == 31)))
    {
        ActualDate = ActualDate.AddDays(1);
    }

    Ну и чтобы не показалось, что привираю, вот соответствующий .aspx:

    <table class="form compact" style="margin: 0 0 0 2em;">
    <tr>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay1" runat="server" Text="1" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay2" runat="server" Text="2" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay3" runat="server" Text="3" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay4" runat="server" Text="4" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay5" runat="server" Text="5" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay6" runat="server" Text="6" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay7" runat="server" Text="7" /></td>
    </tr>
    <tr>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay8" runat="server" Text="8" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay9" runat="server" Text="9" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay10" runat="server" Text="10" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay11" runat="server" Text="11" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay12" runat="server" Text="12" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay13" runat="server" Text="13" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay14" runat="server" Text="14" /></td>
    </tr>
    <tr>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay15" runat="server" Text="15" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay16" runat="server" Text="16" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay17" runat="server" Text="17" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay18" runat="server" Text="18" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay19" runat="server" Text="19" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay20" runat="server" Text="20" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay21" runat="server" Text="21" /></td>
    </tr>
    <tr>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay22" runat="server" Text="22" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay23" runat="server" Text="23" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay24" runat="server" Text="24" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay25" runat="server" Text="25" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay26" runat="server" Text="26" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay27" runat="server" Text="27" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay28" runat="server" Text="28" /></td>
    </tr>
    <tr>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay29" runat="server" Text="29" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay30" runat="server" Text="30" /></td>
        <td><asp:CheckBox ID="RecurringMonthlyEveryDay31" runat="server" Text="31" /></td>
    </tr>
    </table>

    19.07.2007 at 21:50

  • erka says:

    DOM is nothing
    Lanuage: Java

    NamedNodeMap namedNodeMap;
    for (int i = 0; i < nodeList.getLength(); i++) {
      n = nodeList.item(i);
      namedNodeMap = n.getAttributes();
      curName = namedNodeMap.getNamedItem(ATTR_NAME).toString();
      //curName is something like the string "attvalue"
      //and now we substring curName to get the string attvalue
      value = curName.substring(curName.indexOf("\"") + 1,
                curName.lastIndexOf("\""));
    }

    Refactoring:

    for (int i = 0; i < nodeList.getLength(); i++) {
      n = nodeList.item(i);
      namedNodeMap = n.getAttributes();
      value = namedNodeMap.getNamedItem(ATTR_NAME).getNodeValue();
    }

    19.07.2007 at 22:17

  • COTOHA says:

    ерка, ну не опошляй идею конкурса! “don’t trust live variable” - это не дурной код. это просто ошибка в программе. а копи-пейст настолько банален, что им тут хвастаться просто глупо.

    ты сравни это с икаровским проходом по массиву!

    19.07.2007 at 23:27

  • erka says:

    Все это smell code. Так что замечание не принимается. Лучше еще давай пример :)

    19.07.2007 at 23:36

  • COTOHA says:

    > Ааааааааааа, народ. Вы разрываете мне моск!
    тю. на это тоже можно легко найти ответ. завтра рейдж поднимет один комит…

    19.07.2007 at 23:41

  • rage says:

    Было выдано задание, написать компонет который выводит на страницу аватар пользователя и его имя. На картинке которую заказчик нарисовал для примера - было 10 аватаров, как окаказалось, зря он это сделал. Когда задание было сделано, тестировщики сообщили, что если нужно вывести хотя бы 11 аватаров - то что то не получается. Собственно установить в чём проблема, было не очень сложно.

    protected string GetFriendListTable()
    {
     int CellNr = Convert.ToInt32(txtCellNr.Text);
     IList<FriendList> frList = (IList<FriendList>)Session["FriendsList"];
     int index = frList.Count - CellNr;
     if (index == 1)
     {
      return  string.Format(
         "<table>" +
         "<tr align='center'>{0}</tr>" +
         "</table>",
         FormTableCell(frList[CellNr + 0].FriendID, GetFirstLAstName(frList[CellNr + 0].FriendID))
       );
     }

     if (index == 2)
     {
      return string.Format(
         "<table>" +
         "<tr align='center'>{0}{1}</tr>" +
         "</table>",
         FormTableCell(frList[CellNr + 0].FriendID, GetFirstLAstName(frList[CellNr + 0].FriendID)),
         FormTableCell(frList[CellNr + 1].FriendID, GetFirstLAstName(frList[CellNr + 1].FriendID))
      );
     }
     if (index == 3)
     {
      return string.Format(
        "<table>" +
        "<tr align='center'>{0}{1}{2}</tr>" +
        "</table>",
        FormTableCell(frList[CellNr + 0].FriendID, GetFirstLAstName(frList[CellNr + 0].FriendID)),
        FormTableCell(frList[CellNr + 1].FriendID, GetFirstLAstName(frList[CellNr + 1].FriendID)),
        FormTableCell(frList[CellNr + 2].FriendID, GetFirstLAstName(frList[CellNr + 2].FriendID)));
     }

     if (index == 4)
     {
       return string.Format(
         "<table>" +
         "<tr align='center'>{0}{1}</tr>" +
         "</table>" +
         "<table>" +
         "<tr align='center'>{2}{3}</tr>" +
         "</table>",
          FormTableCell(frList[CellNr + 0].FriendID, GetFirstLAstName(frList[CellNr + 0].FriendID)),
          FormTableCell(frList[CellNr + 1].FriendID, GetFirstLAstName(frList[CellNr + 1].FriendID)),
          FormTableCell(frList[CellNr + 2].FriendID, GetFirstLAstName(frList[CellNr + 2].FriendID)),
          FormTableCell(frList[CellNr + 3].FriendID, GetFirstLAstName(frList[CellNr + 3].FriendID)));
     }

     if (index == 5)
     {
       return string.Format(
         "<table>" +
         "<tr align='center'>{0}{1}{2}</tr>" +
         "</table>" +
         "<table>" +
         "<tr align='center'>{3}{4}</tr>" +
         "</table>",
         FormTableCell(frList[CellNr + 0].FriendID, GetFirstLAstName(frList[CellNr + 0].FriendID)),
         FormTableCell(frList[CellNr + 1].FriendID, GetFirstLAstName(frList[CellNr + 1].FriendID)),
         FormTableCell(frList[CellNr + 2].FriendID, GetFirstLAstName(frList[CellNr + 2].FriendID)),
         FormTableCell(frList[CellNr + 3].FriendID, GetFirstLAstName(frList[CellNr + 3].FriendID)),
         FormTableCell(frList[CellNr + 4].FriendID, GetFirstLAstName(frList[CellNr + 4].FriendID)));
     }

     if (index == 6)
     {
       return string.Format(