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

    • ava kegdan 25.06.2013 19:00 # 0

      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
      Ответить

      • ava scriptin 25.06.2013 19:04 # +5

        >Catch убрал
        ВНЕЗАПНО AccountManager.CurrentManager.CurrentSub scription.Name ОКАЗАЛОСЬ NULL IsValid БРОСАЕТ InvalidSubscriptionException КРОВЬ КИШКИ БУДЕШЬ ЗНАТЬ КАК УБИРАТЬ TRY/CATCH
        Ответить

        • ava kegdan 25.06.2013 19:13 # 0

          Эээ, мне казалось, что если null он сразу выдаст false и не будет проверять дальнейшие условия
          Ответить

          • ava santa_microbe 25.06.2013 19:24 # 0

            Если AccountManager.CurrentManager.CurrentSub scription == null, то сразу выдаст false это верно, но вам же говорят о свойстве Name, а вот там будет кровь, кишки и все остальное.
            Ответить

            • ava kegdan 25.06.2013 19:41 # 0

              А, в этом вопрос. Ну, имхо, isValid должно выдавать только булеан и никаких исключений (ибо здравый смысл). Хотя есть код не свой, и там может всплыть Exception - тогда отлов ошибки придется оставить. Печаль
              Ответить

            • ava kegdan 25.06.2013 19:50 # 0

              Тем более IsValid в данном случае не метод, а булеан поле - он может выдать либо true либо false и никаких нулов и исключений
              Ответить

              • ava scriptin 25.06.2013 20:44 # 0

                А если там геттер, а не обычное поле?
                Ответить

                • ava kegdan 25.06.2013 21:58 # +2

                  Тогда try должен быть в этом гетере (если все совсем плохо). Программа не должен при обращении к полю обьекта вылетать с эксепшеном
                  Ответить

                  • ava santa_microbe 26.06.2013 06:13 # 0

                    А собственно почему не должна, гетер по сути своей просто метод и в нем может возникнуть exception, и не всегда мы будем знать что делать с этим exception, соответственно мы должны передать его дальше
                    Ответить

                    • ava kegdan 26.06.2013 07:23 # 0

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

                      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. Ибо здравый смысл
                      Ответить

                      • ava TauSigma 03.07.2013 20:47 # 0

                        Разработчики 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;
		}
	}
}
                        Ответить

                        • ava bormand 03.07.2013 21:28 # +1

                          Ну вообще говоря, тут лучше бы смотрелся метод GetResult, чем этот чорномагической геттер, который мало того что может надолго повиснуть, ожидая окончания работы таска, так еще и экцепшн, вылетевший при работе таска перевбрасывает. Неприлично все-таки геттеры настолько абузить. А MS как всегда отличился, забивая на свои же гайдлайны.
                          Ответить

                        • ava kegdan 04.07.2013 12:46 # +1

                          В культуре коддинга я на MS не оглядываюсь. Код должен подчиняться законам логики. Тут вообще set не нужен - нужно было метод сделать и не париться
                          Ответить

                          • ava TauSigma 04.07.2013 16:54 # 0

                            С этими тасками вообще очень забавный код выходит.
                            К примеру вот такой код:
                            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; } }
                            Ответить

                            • ava bormand 04.07.2013 16:58 # 0

                              > 2097152
                              Wodoo magic.
                              Ответить

                              • ava kegdan 04.07.2013 17:06 # 0

                                Constant magic. Нас в универе за такую магию бьют)
                                Ответить

                                • ava TauSigma 04.07.2013 17:23 # 0

                                  Я тоже сначала подумал что это неких ХРезулт. Но всё оказалось намного прозаичней:
                                  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...
                                  Ответить

                                  • ava bormand 04.07.2013 17:25 # +1

                                    > Может, стоит разработчикам рассказать про енумы
                                    Да хотя бы в хексе это писали бы... В десятичном это смотрится ужасно.
                                    Ответить

                                    • ava kegdan 04.07.2013 17:31 # 0

                                      Это какие то магические винапишные константы?
                                      Зачем они вообще нужны? Почему их не описать статично в каком нибудь общедоступном классе?

                                      Я чувствую, что грибы были несвежие)
                                      Ответить

                                    • ava TauSigma 04.07.2013 18:02 # 0

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

                                      • ava TauSigma 04.07.2013 18:05 # 0

                                        Но кол-во посланных лучей поноса разработчикам - доставляет :)
                                        Ответить

                                  • ava kegdan 04.07.2013 17:36 # 0

                                    такое ощущение, что программист хотел написать не

                                    internal const int TASK_STATE_FAULTED = 2097152;

                                    а

                                    // 2097152 is TASK_STATE_FAULTED

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

                                    Так оно еще мало того internal, но что то мне подсказывает, что везде в сборке стоит 2097152
                                    Ответить

                  • ava eth0 26.06.2013 06:17 # +5

                    > в этом гетере
                    "В этой" же, женский род, как-никак.
                    Ответить

                    • ava scriptin 26.06.2013 18:27 # 0

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

      • ava Eugene 26.06.2013 09:45 # 0

        В целом так, но думаю, что вот это:
        someShit.Name.ToLower().Contains("free" ) ||someShit.Name.ToLower().Contains("trial")

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

        менее производительно - это очевидно, но зато более читаемо
        Ответить

        • ava anonimb84a2f6fd141 26.06.2013 10:15 # 0

          Еще set сделай, чтобы O(1). А вообще, статический код обычно краткостью не отличается, а яваговно так тем более.
          Ответить

        • ava kegdan 26.06.2013 10:18 # −1

          Это не сработает - имя там выглядит скорее как "SomeShitTrialSomeShitFreeByMicrosof t", а он будет принимать только "trial" и "free"

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

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