Си диез / Говнокод #14509 Ссылка на оригинал

0

  1. 1
  2. 2
  3. 3
  4. 4
  5. 5
  6. 6
  7. 7
  8. 8
  9. 9
  10. 10
  11. 11
  12. 12
  13. 13
  14. 14
  15. 15
  16. 16
  17. 17
  18. 18
  19. 19
  20. 20
  21. 21
  22. 22
  23. 23
  24. 24
  25. 25
  26. 26
  27. 27
  28. 28
  29. 29
//Невероятные приключения Microsoft'а в Индии:

        private string ExtractHttpVerb(XmlDocument configDOM)
        {
            string httpVerb;

            string hv = IfExistsExtract(configDOM, "/Config/method", "2");

            switch (hv)
            {
                case "0":
                    httpVerb = HttpVerbs[0];
                    break;

                case "1":
                    httpVerb = HttpVerbs[1];
                    break;

                case "2":
                    httpVerb = HttpVerbs[2];
                    break;

                default:
                    httpVerb = HttpVerbs[2];
                    break;
            }

            return httpVerb;
        }

Запостил: HellMaster_HaiL HellMaster_HaiL, (Updated )

Комментарии (33) RSS

    • ava neeedle # Ссылка на оригинал 0 ⇈

      Ну во первых возвращать HttpVerbs а не строку.
      Во вторых даже если строку, то ну еж-то в /Config/method находится число?
      Ответить

        • ava neeedle # Ссылка на оригинал 0 ⇈

          я имел ввиду вместо string, в объявлении метода, поставить HttpVerbs.
          И вернуть уже элемент перечисления а не какую-то строку.
          Ответить

      • ava HellMaster_HaiL # Ссылка на оригинал 0 ⇈

        В "/Config/method" непременно находится число.
        Говнокод тут в использовании switch.

        HttpVerbs - это массив строк, метод должен вернуть одну строку из этого массива по номеру, указанному в "/Config/method".
        Ответить

        • ava neeedle # Ссылка на оригинал 0 ⇈

          IfExistsExtract(configDOM, "/Config/method", "2") вернет строку, как вы собираетесь использовать ее в качестве индекса?

          >HttpVerbs - это массив строк
          Это перечисление.
          Ответить

          • ava roman-kashitsyn # Ссылка на оригинал 0 ⇈

            Хм, это очень сложный вопрос. Наверное, просто преобразовать в число не выйдет.
            Ответить

            • ava neeedle # Ссылка на оригинал 0 ⇈

              Ну можно, распарсить... Хотя особо от этого код не похорошеет.

              try {
  return HttpVerbs[int.TryParse(hv)];
}
catch(SomeExeption ex) {
}
              Ответить

          • ava HellMaster_HaiL # Ссылка на оригинал 0 ⇈

            >Это перечисление.
            private static readonly string[] HttpVerbs = new[] { "GET", "HEAD", "POST" };
            Хотя особого значения не имеет.

            >вернет строку, как вы собираетесь использовать ее в качестве индекса?
            Как-то так:
            var hv = IfExistsExtract(configDOM, "/Config/method", "2");
int hvi;
if (!int.TryParse(hv, out hvi))
{
   hvi = 2;
}
var httpVerb = HttpVerbs[hvi];
return httpVerb;
            Ответить

            • ava neeedle # Ссылка на оригинал 0 ⇈

              Ну предположим вы вот так обработали.
              try {
int index = int.Parse(IfExistsExtract(configDOM, "/Config/method", "2"));
catch(){}
catch(){}

              return HttpVerbs[index];
              Только у метода parse 3 исключения возможных, плюс IndexOutOfRange, плюс возможные исключения метода ifexistextract.
              Ваш, правильный код, не будет коротеньким.
              Ну разве что вы пишите только для идеального случая.
              Ответить

              • ava HellMaster_HaiL # Ссылка на оригинал 0 ⇈

                Не все, что длинно - говнокод.
                Не все. что коротко - совершенно.
                Бороться за строки кода - это глупость, в угоду расширяемости и отказоустойчивости.
                Добавить проверки на исключения - это песня другой оперы.

                Я же сказал, что говнокодом в данном случае я считаю оператор switch.
                Ответить

            • ava bormand # Ссылка на оригинал 0 ⇈

              У меня только один вопрос. Какого хуя почему в конфиге лежит неведомый порядковый номер HTTP метода, который потом все равно отображают в нормальный HTTP verb?

              Правильный вариант:
              String verb = IfExistsExtract(configDOM, "/Config/method", "POST");
if (allowedVerbs.contains(verb))
    return verb;
return "POST";
              Ответить

              • ava neeedle # Ссылка на оригинал 0 ⇈

                Есть мнение, что можно как-то сделать именно вот так. И я бы начал именно с этого а не с того, какой метод использовать для парсинга неведомого числа.
                Ответить

                • ava bormand # Ссылка на оригинал 0 ⇈

                  Ну или с enum'ом, как ты и предлагал:
                  enum HttpVerb {
    HEAD,
    GET,
    POST
}

private HttpVerb extractHttpVerb(XmlDocument configDOM) {
    try {
        return HttpVerb.valueOf(
            IfExistsExtract(configDOM, "/Config/method", "").toUpperCase());
    } catch (IllegalArgumentException e) {
        return HttpVerb.POST;
    }
}
                  Ответить

              • ava HellMaster_HaiL # Ссылка на оригинал 0 ⇈

                > У меня только один вопрос. Какого хуя почему в конфиге лежит неведомый порядковый номер метода, который потом все равно отображают в нормальный HTTP verb?
                На этот вопрос Вам ответят разработчики MS BizTalk, ибо конфиг приходит из его "интерфейса" в котором свой, отдельный, справочник вида:
                {0, "GET"}, {1, "POST"}, {2, "PUT"} и так далее.
                Конфиг в данном случае - это ресурс, на который мы влиять не можем.

                В данном случае return IfExistsExtract(configDOM, "/Config/method", "POST");
                Вернет либо строку из "/Config/method" (а это может быть "0", "1", "2"...."N"), либо строку "POST".
                В первом случае - это беда-печаль.
                Ответить

                • ava bormand # Ссылка на оригинал 0 ⇈

                  > Конфиг в данном случае - это ресурс, на который мы влиять не можем.
                  > разработчики MS BizTalk
                  Мда, так вот где индусы порылись 😉
                  Ответить

                • ava bormand # Ссылка на оригинал 0 ⇈

                  Ну тогда как-то так...
                  private static Map<String, String> httpVerbs = new HashMap<String, String>();
{ // статик конструктор
    httpVerbs.put("0", "HEAD");
    httpVerbs.put("1", "GET");
    httpVerbs.put("2", "POST");
    ...
}

private String ExtractHttpVerb(XmlDocument configDOM) {
    String verb = IfExistsExtract(configDOM, "/Config/method", "");
    verb = httpVerbs.get(verb);
    if (verb != null)
        return verb;
    return "POST";
}
                  P.S. Сорри, я код на жабе написал, спутал язык, только сейчас посмотрел, что тут речь о c# :))))
                  Ответить

                  • ava HellMaster_HaiL # Ссылка на оригинал 0 ⇈

                    Хэш - тоже вариант, конечно, но мне не особо нравится.
                    Ведь список методов, по сути, может быть тоже сторонним ресурсом. пришедшим из бездны.
                    В идеале метода ExtractHttpVerb должен принимать xml и IEnumerator (список методов) и работать уже с ними.
                    Еще более кошерно, естественно, получить в конфиге чистую строку метода, но это не всегда возможно.
                    Ответить

                    • ava bormand # Ссылка на оригинал 0 ⇈

                      > IEnumerator (список методов)
                      Да тоже как-то топорно - не понятно, откуда брать дефолт. Хардкодить "2" или "POST" как-то некрасиво, раз уж мап теперь приходит извне. Передавать дефолт третьим параметром тоже как-то не айс.

                      Может быть стоит запилить отдельный универсальный класс в духе MapWithDefault, которому в конструкторе дают мап и дефолт (например добыв все это из недр бизтолка). А метод get этого класса возвращает значение из мапа, или дефолт.

                      Так мы избавимся от непонятного "2" и абстрагируемся от добывания этого самого мапа. А код выродится в httpVerbs.get(IfExistsExtract(configDOM, "/Config/method", "")
                      Ответить

                • ava neeedle # Ссылка на оригинал 0 ⇈

                  Тогда идеальным будет перенести этот конфиг в какой-то словарь
                  dictionary<string, Httpverbs> и по ключу искать, а не парсить потом исправлять кучу ошибок или кидать их дальше по слоям.
                  И код укоротится еще больше:
                  string index = IfExistsExtract(configDOM, "/Config/method", "2");
returnHttpVerbsDictionary[index];


                  А вообще класс с таким словарем должен быть, ИМХО, я бы сделал.
                  Ответить

                  • ava HellMaster_HaiL # Ссылка на оригинал 0 ⇈

                    Ваш вариант тоже не лишен исключений - это когда в returnHttpVerbsDictionary нет такого ключа.

                    Еще, кстати, интересно будет протестировать, что будет работать быстрее:
                    парсинг + поиск по числовому индексу в массиве
                    или поиск по строковому ключу словаря
                    Ответить

                    • ava neeedle # Ссылка на оригинал 0 ⇈

                      тоже самое что индекс в массиве.
                      Так или иначе это одна единственная ошибка, которую легко обработать.
                      Ответить

              • ava HellMaster_HaiL # Ссылка на оригинал 0 ⇈

                Вам покоя не дает это перечисление 🙂
                Это совершенно не это. Нэймспэйс System.Web.Mvc не используется.
                Да и по сути такого жесткого перечисления не нужно. Я использую, к примеру, только GET и POST. Остальные все убрал как и с конфига, так и из кода.
                Ответить

Добавить комментарий

Помни, guest, за тобой могут следить!

    А не использовать ли нам bbcode?

    ↑
    8
    ↓