1. C# / Говнокод #13229

    +133

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    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
    private bool IsSubscriptionFree()
    {
        try
        {
            if (AccountManager.CurrentManager.CurrentSubscription != null)
            {
                if (AccountManager.CurrentManager.CurrentSubscription.IsValid)
                {
                    if (AccountManager.CurrentManager.CurrentSubscription.Name.ToLower().Contains("free") ||
                        AccountManager.CurrentManager.CurrentSubscription.Name.ToLower().Contains("trial")
                    )
                    {
                        return true;
                    }
                }
                return false;
            }
            return false;
        }
        catch (Exception)
        {
            return false;
        }
    }

    Запостил: Eugene, 25 Июня 2013

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

    • private bool IsSubscriptionFree()
      {

      var someShit = AccountManager.CurrentManager.CurrentSub scription;

      return (someShit != null) && (someShit.IsValid) && (someShit.Name.ToLower().Contains("free" )
      ||someShit.Name.ToLower().Contains("tria l"));


      }

      Как то так?
      В задачу не вникал, но вытаскивать из текста trial оно или нет - это круто

      Catch убрал из соображений, что ошибка тут может быть только kind of magic
      Ответить
      • >Catch убрал
        ВНЕЗАПНО AccountManager.CurrentManager.CurrentSub scription.Name ОКАЗАЛОСЬ NULL IsValid БРОСАЕТ InvalidSubscriptionException КРОВЬ КИШКИ БУДЕШЬ ЗНАТЬ КАК УБИРАТЬ TRY/CATCH
        Ответить
        • Эээ, мне казалось, что если null он сразу выдаст false и не будет проверять дальнейшие условия
          Ответить
          • Если AccountManager.CurrentManager.CurrentSub scription == null, то сразу выдаст false это верно, но вам же говорят о свойстве Name, а вот там будет кровь, кишки и все остальное.
            Ответить
            • А, в этом вопрос. Ну, имхо, isValid должно выдавать только булеан и никаких исключений (ибо здравый смысл). Хотя есть код не свой, и там может всплыть Exception - тогда отлов ошибки придется оставить. Печаль
              Ответить
            • Тем более IsValid в данном случае не метод, а булеан поле - он может выдать либо true либо false и никаких нулов и исключений
              Ответить
              • А если там геттер, а не обычное поле?
                Ответить
                • Тогда try должен быть в этом гетере (если все совсем плохо). Программа не должен при обращении к полю обьекта вылетать с эксепшеном
                  Ответить
                  • А собственно почему не должна, гетер по сути своей просто метод и в нем может возникнуть exception, и не всегда мы будем знать что делать с этим exception, соответственно мы должны передать его дальше
                    Ответить
                    • В программировании важен здравый смысл. При обращению к полю обьект не должен менять своих свойств и делать что то страшное. Геттеры созданы для предобработки данных, ну, например:

                      private double _angle;
                      public double Angle
                      {
                      set
                      {


                      var k = value;
                      if (Math.Abs(k) > 360) k = k % 360;
                      if (k < 0) k += 360;
                      _angle = k;
                      }
                      get { return _angle; }
                      }


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

                      В данном конкретном случае эксепшен означает, что обьект не валиден, и метод IsValid должен вернуть false. Ибо здравый смысл
                      Ответить
                      • Разработчики M$ готовы поспорить.
                        System.Threading.Tasks.Task<TResult>:
                        public class Task<TResult> : Task
                        {
                        	public TResult Result
                        	{
                        		get
                        		{
                        			if (!base.IsCompleted)
                        			{
                        				Debugger.NotifyOfCrossThreadDependency();
                        				base.Wait();
                        			}
                        			base.ThrowIfExceptional(!this.m_resultWasSet);//Тут может быть AggregateException
                        			return this.m_result;
                        		}
                        	}
                        }
                        Ответить
                        • Ну вообще говоря, тут лучше бы смотрелся метод GetResult, чем этот чорномагической геттер, который мало того что может надолго повиснуть, ожидая окончания работы таска, так еще и экцепшн, вылетевший при работе таска перевбрасывает. Неприлично все-таки геттеры настолько абузить. А MS как всегда отличился, забивая на свои же гайдлайны.
                          Ответить
                        • В культуре коддинга я на MS не оглядываюсь. Код должен подчиняться законам логики. Тут вообще set не нужен - нужно было метод сделать и не париться
                          Ответить
                          • С этими тасками вообще очень забавный код выходит.
                            К примеру вот такой код:
                            Task<DataItem> task=new Task<DataItem>(()=> { ... });
                            task.Start();
                            ...
                            task.Wait();
                            if(!task.IsFaulted) return task.Result;

                            Всё равно может выбросить исключение AggregateException при обращении к проперти Result.

                            Для интереса, содержимое проперти IsFauted: (Сравните с кодом Result выше)
                            // System.Threading.Tasks.Task
                            /// <summary>Gets whether the <see cref="T:System.Threading.Tasks.Task" /> completed due to an unhandled exception.</summary>
                            /// <returns>true if the task has thrown an unhandled exception; otherwise false.</returns>
                            public bool IsFaulted { get { return (this.m_stateFlags & 2097152) != 0; } }
                            Ответить
                            • > 2097152
                              Wodoo magic.
                              Ответить
                              • Constant magic. Нас в универе за такую магию бьют)
                                Ответить
                                • Я тоже сначала подумал что это неких ХРезулт. Но всё оказалось намного прозаичней:
                                  internal const int TASK_STATE_FAULTED = 2097152;
                                  internal void FinishStageTwo()
                                  {
                                  	this.AddExceptionsFromChildren();
                                  	int num;
                                  	if (this.ExceptionRecorded)
                                  	{
                                  		num = 2097152;
                                  	}
                                  	else
                                  	{
                                  		if (this.IsCancellationRequested && this.IsCancellationAcknowledged)
                                  		{
                                  			num = 4194304;
                                  		}
                                  		else
                                  		{
                                  			num = 16777216;
                                  		}
                                  	}
                                  	Interlocked.Exchange(ref this.m_stateFlags, this.m_stateFlags | num);
                                  	this.SetCompleted();
                                  	this.DeregisterCancellationCallback();
                                  	this.FinishStageThree();
                                  }

                                  Может, стоит разработчикам рассказать про енумы и атрибут Flags...
                                  Ответить
                                  • > Может, стоит разработчикам рассказать про енумы
                                    Да хотя бы в хексе это писали бы... В десятичном это смотрится ужасно.
                                    Ответить
                                    • Это какие то магические винапишные константы?
                                      Зачем они вообще нужны? Почему их не описать статично в каком нибудь общедоступном классе?

                                      Я чувствую, что грибы были несвежие)
                                      Ответить
                                    • Если учесть, что в куче #Blob метаданных, константы написаны в массивах байт и преобразуются в нужные типы (http://msdn.microsoft.com/en-us/library/ms232600.aspx) исходя из сигнатуры поля/типа/проперти и т.п. соответсвующих таблиц в потоке #~, то отображение константы в десятичном виде лежит полностью на разработчиках ILSpy'я (http://ilspy.net/).
                                      Ответить
                                      • Но кол-во посланных лучей поноса разработчикам - доставляет :)
                                        Ответить
                                  • такое ощущение, что программист хотел написать не

                                    internal const int TASK_STATE_FAULTED = 2097152;

                                    а

                                    // 2097152 is TASK_STATE_FAULTED

                                    один фиг не использует поле

                                    Так оно еще мало того internal, но что то мне подсказывает, что везде в сборке стоит 2097152
                                    Ответить
                  • > в этом гетере
                    "В этой" же, женский род, как-никак.
                    Ответить
                    • Объясните, почему женский. Я употребляю мужской, потому что понимаю "геттер" как "метод".
                      Ответить
      • В целом так, но думаю, что вот это:
        someShit.Name.ToLower().Contains("free" ) ||someShit.Name.ToLower().Contains("trial")

        я бы сделал вот так:
        new []{"free","trial"}.Contains(someShit.Name, StringComparer.OrdinalIgnoreCase)

        менее производительно - это очевидно, но зато более читаемо
        Ответить
        • Еще set сделай, чтобы O(1). А вообще, статический код обычно краткостью не отличается, а яваговно так тем более.
          Ответить
        • Это не сработает - имя там выглядит скорее как "SomeShitTrialSomeShitFreeByMicrosof t", а он будет принимать только "trial" и "free"

          Никогда не занимался распарсиванием свойств из строки - enum флагов - гораздо удачнее
          Ответить

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