Fix if-else syntax for the case missing OSS support #22

Обединени
SlavekB обедини 1 ревизии от feat/fix-if-else-cond във master преди 5 години
Сътрудник

OSS MIDI API may be missing even if ALSA is enabled.

OSS MIDI API may be missing even if ALSA is enabled.
SlavekB reviewed преди 5 години
SlavekB left a comment
Притежател

Please consider simplifying the code as suggested in the comment.

Please consider simplifying the code as suggested in the comment.
#endif
#ifdef HAVE_OSS_SUPPORT
_seqbufptr=0;
#endif
Притежател

The combination of nested #ifdef and if-else in the code looks hard to read. I propose simplification as follows:

#ifdef HAVE_ALSA_SUPPORT
  if (alsa)
  {
    ((AlsaOut *)device[default_dev])->seqbuf_clean();
    return;
  }
#endif
#ifdef HAVE_OSS_SUPPORT
  {
    _seqbufptr=0;
   return;
  }
#endif
The combination of nested #ifdef and if-else in the code looks hard to read. I propose simplification as follows: ``` #ifdef HAVE_ALSA_SUPPORT if (alsa) { ((AlsaOut *)device[default_dev])->seqbuf_clean(); return; } #endif #ifdef HAVE_OSS_SUPPORT { _seqbufptr=0; return; } #endif ```
Притежател

a simpler version is:

#ifdef HAVE_ALSA_SUPPORT
  if (alsa)
  {
    ((AlsaOut *)device[default_dev])->seqbuf_clean();
    return;
  }
#endif
#ifdef HAVE_OSS_SUPPORT
  _seqbufptr=0;
#endif
a simpler version is: ``` #ifdef HAVE_ALSA_SUPPORT if (alsa) { ((AlsaOut *)device[default_dev])->seqbuf_clean(); return; } #endif #ifdef HAVE_OSS_SUPPORT _seqbufptr=0; #endif ```
Притежател

Yes, of course, this can make the code even simpler. I chose the closed block also for OSS to make it clear if there will be added support for some other sound systems – for example, for Solaris 😸

Yes, of course, this can make the code even simpler. I chose the closed block also for OSS to make it clear if there will be added support for some other sound systems – for example, for Solaris :smile_cat:
Участник
Сътрудник

For multiple sound systems support, is it intend to be runtime switching or build time?

For multiple sound systems support, is it intend to be runtime switching or build time?
Участник
Сътрудник

For multiple sound systems support, is it intend to be runtime switching or build time?

For multiple sound systems support, is it intend to be runtime switching or build time?
Притежател

Please give attention to my review – see comment above.

Please give attention to my review – see comment above.
Участник
Сътрудник

Changed to coding style same as others.

Changed to coding style same as others.
SlavekB closed this pull request преди 5 години
SlavekB deleted branch feat/fix-if-else-cond преди 5 години
SlavekB added this to the R14.0.6 release milestone преди 5 години
Тази заявка за сливане е била обединена като dd6da7345a.
Впишете се за да се присъедините към разговора.
No reviewers
Няма етап
Няма изпълнители
3 участника
Известия
Due Date

No due date set.

Зависимости

No dependencies set.

Reference: TDE/tdelibs#22
Зареждане…
Все още няма съдържание.