Opened 6 years ago

Closed 6 years ago

#11525 closed Patch - Feature (Won't Fix)

program category clean up

Reported by: rwscott@… Owned by:
Priority: minor Milestone: unknown
Component: MythTV - General Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

Go one step farther to prevent mistakes such as invalid values being stored and used. The original patch, bff155970773, changed the category type from a string to an enum to help avoid mistakes in the future. It however meant that a couple of string arrays, and some static int's (added later to fix the fix), _must_ be kept in sync. This just changes the type of future mistakes. This defines the category type in one place only.

Attachments (2)

program-category.patch (3.1 KB) - added by rwscott@… 6 years ago.
Go one step farther to prevent mistakes such as invalid values being stored and used
0001-remove-duplication-and-magic-numbers-in-category-typ.patch (2.7 KB) - added by Karl Egly 6 years ago.
a variation on the patch

Download all attachments as: .zip

Change History (4)

Changed 6 years ago by rwscott@…

Attachment: program-category.patch added

Go one step farther to prevent mistakes such as invalid values being stored and used

Changed 6 years ago by Karl Egly

a variation on the patch

comment:1 Changed 6 years ago by Karl Egly

Find attached my variation on the patch (as discussed back on http://irc.mythtv.org/ircLog/channel/4/2013-04-14:18:00 )

comment:2 Changed 6 years ago by stuartm

Resolution: Won't Fix
Status: newclosed

I'm going to quote myself from IRC:

"stuartm: I can't really argue with the reasoning for the patch just sent to -dev, but I think such macros are as clear as mud :/"

...

"stuartm: I don't know how big a problem it really is, the category types haven't changed in years and no-one is proposing extending them any time soon, the aim of my commit was to avoid future bugs through typos or invalid assumptions such as the Dish EIT one – basically that you could stuff any old value into that field"

In summary, while I appreciate the patch, I think it hurts code readability. The content of that enum is never likely to change. The reason for switching to the enum in the first places was because the category types are used all over the code creating a substantial risk that invalid values might be used. The risk of someone adding to the enum without updating the array/int is insignificant by comparison.

Note: See TracTickets for help on using tickets.