Introduction
In my OpenGL Video Encoder, the constructor for H264Writer
used to be like this before refactoring, the boolean parameter is to indicate to use HEVC codec. What is not apparent is when it is set to false
, what codec is used? H264 or WMV?
H264Writer(const wchar_t* mp3_file, const wchar_t* src_file,
const wchar_t* dest_file, bool hevc,
UINT32 bitrate = 4000000);
To remedy this for clarity, I introduced an enum
called VideoCodec
to replace the boolean. It also make it easy for expansion when I add a third or fourth codec in future.
enum class VideoCodec
{
H264,
HEVC
};
H264Writer(const wchar_t* mp3_file, const wchar_t* src_file,
const wchar_t* dest_file, VideoCodec codec,
UINT32 bitrate = 4000000);
In EnumVideoEncoder
function, I did the same refactoring. When calling the function, which is more clear in its intent to the code reader?
if (H264Writer::EnumVideoEncoder(encoders, Processing::Software, VideoCodec::H264))
Or this?
if (H264Writer::EnumVideoEncoder(encoders, false, false)
Clearly, the winner is the first one. Never abuse boolean to be your 2 value type, use an enum
instead.
In my Outline Text Library, in the TextGradOutlineStrategy
class's Init
method, the last parameter is another example of abusing boolean. It is not clear to anyone except the original owner what gradient type is used for when the SinusoidGradient
is false
.
void Init(
Gdiplus::Color clrText,
Gdiplus::Color clrOutline1,
Gdiplus::Color clrOutline2,
int nThickness,
bool SinusoidGradient);
To fix it, a GradientType
enum
is added to replace the boolean.
enum class GradientType
{
Linear,
Sinusoid
};
void Init(
Gdiplus::Color clrText,
Gdiplus::Color clrOutline1,
Gdiplus::Color clrOutline2,
int nThickness,
GradientType gradType);
Likewise, you should not use a boolean type to represent a gender type. You get my point by now. Even a well-established software behemoth like Microsoft committed the same mistake with MFC CFileDialog. I always had a hard time in reading a CFileDialog
whether it is OpenFileDialog
or SaveFileDialog
without first looking it up in MSDN.
CFileDialog(BOOL bOpenFileDialog,
LPCTSTR lpszDefExt = NULL,
LPCTSTR lpszFileName = NULL,
DWORD dwFlags = OFN_HIDEREADONLY | OFN_OVERWRITEPROMPT,
LPCTSTR lpszFilter = NULL,
CWnd* pParentWnd = NULL );
...
CFileDialog dlg(FALSE);