[Rejected] TVE3 channelType as enum instead of int (1 Viewer)

[tanstaafl]

I stumbled across the CardType enum in TvLibrary.Interfaces and figured it'd be useful to have a ChannelType enum as well.

This patch adds ChannelType.cs in TvLibrary.Interfaces (same location as CardType.cs) and refactors relevant code in TVE3.

I can only test with analog channels, (working), so more testing probably needed before commit (if you even like the idea .


Note on TvBusinessLayer.BusinessLayer.cs:

Base code bunches together ATSC and Analog channels as channelType = 1.
I assumed this was unintentional, so the patch additionally differentiates between AnalogChannel and ATSCChannel like so:

Index: TvEngine3/TVLibrary/TVDatabase/TvBusinessLayer/BusinessLayer.cs
===================================================================
--- TvEngine3/TVLibrary/TVDatabase/TvBusinessLayer/BusinessLayer.cs	(revision 23209)
+++ TvEngine3/TVLibrary/TVDatabase/TvBusinessLayer/BusinessLayer.cs	(working copy)
@@ -287,29 +287,35 @@
 
     public TuningDetail GetChannel(DVBBaseChannel channel)
     {
-      int channelType;
+      ChannelType channelType;
 
+
       if (channel is DVBTChannel)
       {
-        channelType = 4;
+        channelType = ChannelType.DVBT;
       }
       else if (channel is DVBSChannel)
       {
-        channelType = 3;
+        channelType = ChannelType.DVBS;
       }
       else if (channel is DVBCChannel)
       {
-        channelType = 2;
+        channelType = ChannelType.DVBC;
       }
       else if (channel is DVBIPChannel)
       {
-        channelType = 7;
+        channelType = ChannelType.DVBIP;
       }
-      else // must be ATSCChannel  or AnalogChannel
+      else if (channel is ATSCChannel)
       {
-        channelType = 1;
+        channelType = ChannelType.ATSC;
       }
+      else // must be AnalogChannel
+      {
+        channelType = ChannelType.Analog;
+      }

This is my first patch, apologies if I've done something wrong!

/tans

 

[chemelli]

Note on TvBusinessLayer.BusinessLayer.cs:

Base code bunches together ATSC and Analog channels as channelType = 1.

At the current state of 1.1.0 we want to avoid bugs as much as possible, this is why I'm going to reject this patch.
It's a readbility code improvement that doesn't worth the risk.

:sorry:

Simone

 

[tanstaafl]

No worries!

Thanks for looking at it.

/Tanstaafl

 

[tanstaafl]

Actually, what about the BusinessLayer issue (ATSC/Analog == 1)? That's unrelated to the readability improvement suggestion.

/tans

 

[chemelli]

Actually, what about the BusinessLayer issue (ATSC/Analog == 1)? That's unrelated to the readability improvement suggestion.

/tans

This was the main reason for the reject

Simone