Вложенные попытки-ловит или есть лучший способ? - PullRequest
0 голосов
/ 03 ноября 2010

В приведенном ниже методе есть многочисленные операторы case (многие из них были удалены), которые делают вызовы в классы Manager.Например, первый вызывает ApplicationManager.GetByGUID.Каждый раз, когда используется класс «менеджер», происходят проверки безопасности.

Проблема: у меня есть объекты, которые могут быть разрешены для некоторых из них, но не для всех.Поэтому, когда этот метод будет запущен, если один из них выйдет из строя, он сгенерирует исключение безопасности и вылетит весь отчет.

Кто-то предложил мне, чтобы я мог просто бросать блоки try-catch вокруг каждого случая, ночем больше я читаю, тем больше чувствую, что это может быть небрежно.По общему признанию, я не очень осведомлен об исключениях ... Я надеялся, что кто-то мог бы предложить способ сделать это с большей утонченностью ... Мне нужно иметь возможность вернуть хорошие данные и игнорировать те, которые генерируют исключения безопасности ....или, может быть, в этом случае все в порядке? -1005 *

Надеюсь, это имеет смысл ... спасибо


private string GetLookup(string value, string type)
{
    MySqlConnection mconn = new MySqlConnection(ConfigurationSettings.AppSettings["UnicornConnectionString_SELECT"]);

    try
    {
        mconn.Open();

        lock (reportLookups)
        {
            if (reportLookups.ContainsKey(type+value))
                return reportLookups[type+value].ToString();
            else if (reportLookups.ContainsKey(value))
                return reportLookups[value].ToString();
            else
            {
                switch (type)
                {
                    case "ATTR_APPLICATIONNAME":
                        if (value != Guid.Empty.ToString())
                        {
                            reportLookups.Add(type + value, applicationManager.GetByGUID(value).Name);
                        }
                        else
                        {
                            reportLookups.Add(type + value, "Unknown");
                        }
                        mconn.Close();
                        return reportLookups[type + value].ToString();
                        break;
                    case "ATTR_CITYNAME":
                        reportLookups.Add(type + value, UMConstantProvider.UMConstantProvider.GetConstant<UMString64>(int.Parse(value), UMMetricsResourceLibrary.Enumerations.ConstantType.CITY_NAME, ref mconn));
                        mconn.Close();
                        return reportLookups[type + value].ToString();
                        break;
                    case "ATTR_COUNTRYNAME":
                        reportLookups.Add(type + value, UMConstantProvider.UMConstantProvider.GetConstant<UMString2>(int.Parse(value), UMMetricsResourceLibrary.Enumerations.ConstantType.COUNTRY_NAME, ref mconn));
                        mconn.Close();
                        return reportLookups[type + value].ToString();
                        break;
                    case "ATTR_ITEMDURATION":

                        MediaItem mi = mediaItemManager.GetMediaItemByGUID(value);
                        if (mi.MediaItemTypeID == (int)MediaItemType.ExternalVideo || mi.MediaItemTypeID == (int)MediaItemType.ExternalAudio)
                        {
                            reportLookups.Add(type + value, mediaItemManager.GetMediaItemByGUID(value).ExternalDuration);
                            mconn.Close();
                            return reportLookups[type + value].ToString();
                        }
                        else
                        {

                            List<BinaryAsset> bins = fileSystemManager.GetBinaryAssetsByMediaItemGuid(value, mi.DraftVersion);
                            var durationasset = from d in bins
                                                where d.Duration != 0
                                                select d.Duration;

                            if (durationasset.Count() > 0)
                            {

                                reportLookups.Add(type + value, durationasset.ToList()[0]);

                            }
                            else
                            {
                                reportLookups.Add(type + value, 0);
                                mconn.Close();
                                return reportLookups[type + value].ToString();
                            }


                        }

                        break;
                }
            }
            return string.Empty;
        }
    }
    finally 
    {
        mconn.Close();
    }
}

Ответы [ 2 ]

3 голосов
/ 03 ноября 2010

Как правило, исключения должны указывать, что что-то пошло не так. Если вы ожидаете исключения во время типичного прогона этого метода, вы должны изменить свои API, чтобы избежать этого исключения:

if (mediaItemManager.CanAccessMediaItem(value))
{
    MediaItem mi = mediaItemManager.GetMediaItemByGUID(value);
    ....
}

Вот моя быстрая попытка преобразовать этот код во что-то более разумное:

private string GetLookup(string value, string type)
{
    var lookupKey = type + value;                        
    using (MySqlConnection mconn = new MySqlConnection(ConfigurationSettings.AppSettings["UnicornConnectionString_SELECT"]))
    {
        mconn.Open();
        lock (reportLookups)
        {
            if (reportLookups.ContainsKey(lookupKey))
            {
                return reportLookups[lookupKey].ToString();
            }
            var value = GetLookupValue(type, value);
            reportLookups[lookupKey] = value;
            return value;
        }
    }
}

private string GetLookupValue(string type, string value)
{
    switch (type)
    {
        case "ATTR_APPLICATIONNAME":
            return value == Guid.Empty.ToString() 
                ? "Unknown"
                : applicationManager.CanGetByGUID(value)
                    ? applicationManager.GetByGUID(value).Name
                    : string.Empty;
        case "ATTR_CITYNAME":
            return UMConstantProvider.UMConstantProvider.GetConstant<UMString64>(int.Parse(value), UMMetricsResourceLibrary.Enumerations.ConstantType.CITY_NAME, ref mconn);
        case "ATTR_COUNTRYNAME":
            return UMConstantProvider.UMConstantProvider.GetConstant<UMString2>(int.Parse(value), UMMetricsResourceLibrary.Enumerations.ConstantType.COUNTRY_NAME, ref mconn);
        case "ATTR_ITEMDURATION":
            if(mediaItemManager.CanGetMediaItemByGUID(value)) {
                MediaItem mi = mediaItemManager.GetMediaItemByGUID(value);
                if (mi.MediaItemTypeID == (int)MediaItemType.ExternalVideo || mi.MediaItemTypeID == (int)MediaItemType.ExternalAudio)
                {
                    return mediaItemManager.GetMediaItemByGUID(value).ExternalDuration;
                }
                else
                {
                    List<BinaryAsset> bins = fileSystemManager.GetBinaryAssetsByMediaItemGuid(value, mi.DraftVersion);
                    var durationasset = from d in bins
                                        where d.Duration != 0
                                        select d.Duration;
                    return durationasset.FirstOrDefault() ?? "0";
                }
            }
            else 
            {
                return string.Empty;
            }
        default:
            return string.Empty;
    }
}

Поскольку я не понимаю всей области действия этого кода, я, вероятно, упростил некоторые его аспекты, но вы можете видеть, что здесь необходимо выполнить много рефакторинга. В будущем, возможно, вы захотите выполнить какой-то код по http://refactormycode.com/,, пока не привыкнете к использованию передовых методов.

0 голосов
/ 03 ноября 2010

Где-то у вас будет такой код:

foreach(Request req in allRequests)
{
   Reply result = MakeReply(req);
   WriteReply(result);
}

Преврати это в:

foreach(Request req in allRequests)
{
   Reply result;
   try
   {
      result = CreateReply(req);
   }
   catch(SecurityException ex)
   {
      result = CreateReplyUnauthorized();
   }
   catch(Exception ex) // always the last
   {
      LogException(ex); // for bug hunting

      // Don't show the exception to the user - that's a security risk
      result = CreateReplySystemError();
   }

   WriteReply(result);
}

Возможно, вы захотите поместить try-catch в отдельную функцию, так как тело вашего цикла foreach становится большим, как только вы поймаете несколько типов исключений.

StriplingWarrior также прав в своем ответе: «Исключения должны указывать, что что-то пошло не так». Пусть они распространяются на основной цикл и показывают их там.

...