patterncppMinor
Monitor and video mode querying
Viewed 0 times
modequeryingvideomonitorand
Problem
I am working on a large project that requires information about the monitor(s). Please don't be confused by the organization of the following structs. The design intent is to be cross-platform, so they don't contain any operating system-dependent types. Currently, I am working on the Windows implementation of the following functions.
Please bear with me as I am a beginner at C++. Even though I have been programming with it for a while now, I am completely self taught, and my understanding isn't comprehensive.
Is there anything I could be doing differently, or maybe better? Also, is there anything I am not understanding correctly? I'm open to any and all suggestions, tips, and criticisms.
```
/**
* Monitor structure.
*/
struct tsMonitor
{
/**
* Windows specific data.
*/
typedef struct SWin32 { std::string adapterName; std::string displayName; } _win32;
std::string adapterName; // Name of the adapter (GPU).
std::string displayName; // Actual monitor name.
_win32 win32; // Windows adapter and display names. Fuck you Windows and your stupid "DeviceName" vs "DeviceString".
unsigned short widthMM, heightMM; // Physical dimensions in millimeters.
};
/**
* Video mode structure.
*/
struct tsVideoMode
{
unsigned short width, height; // Dimensions in pixels.
unsigned char refreshRate; // Refresh rate.
unsigned char colorDepth; // Number of bits per pixel.
};
std::vector tsGetMonitors()
{
std::vector returnVal;
for(unsigned char adapterIndex = 0;; adapterIndex++)
{
DISPLAY_DEVICE adapter;
ZeroMemory(&adapter, sizeof(DISPLAY_DEVICE));
adapter.cb = sizeof(DISPLAY_DEVICE);
if(!EnumDisplayDevices(NULL, adapterIndex, &adapter, 0)) break;
if(!(adapter.StateFlags & DISPLAY_DEVICE_ACTIVE)) continue;
for(unsigned char displayIndex = 0;; displayIndex++)
{
DISPLAY_DEVICE display;
Please bear with me as I am a beginner at C++. Even though I have been programming with it for a while now, I am completely self taught, and my understanding isn't comprehensive.
Is there anything I could be doing differently, or maybe better? Also, is there anything I am not understanding correctly? I'm open to any and all suggestions, tips, and criticisms.
```
/**
* Monitor structure.
*/
struct tsMonitor
{
/**
* Windows specific data.
*/
typedef struct SWin32 { std::string adapterName; std::string displayName; } _win32;
std::string adapterName; // Name of the adapter (GPU).
std::string displayName; // Actual monitor name.
_win32 win32; // Windows adapter and display names. Fuck you Windows and your stupid "DeviceName" vs "DeviceString".
unsigned short widthMM, heightMM; // Physical dimensions in millimeters.
};
/**
* Video mode structure.
*/
struct tsVideoMode
{
unsigned short width, height; // Dimensions in pixels.
unsigned char refreshRate; // Refresh rate.
unsigned char colorDepth; // Number of bits per pixel.
};
std::vector tsGetMonitors()
{
std::vector returnVal;
for(unsigned char adapterIndex = 0;; adapterIndex++)
{
DISPLAY_DEVICE adapter;
ZeroMemory(&adapter, sizeof(DISPLAY_DEVICE));
adapter.cb = sizeof(DISPLAY_DEVICE);
if(!EnumDisplayDevices(NULL, adapterIndex, &adapter, 0)) break;
if(!(adapter.StateFlags & DISPLAY_DEVICE_ACTIVE)) continue;
for(unsigned char displayIndex = 0;; displayIndex++)
{
DISPLAY_DEVICE display;
Solution
I have some general comments as I'm not too familiar with the Windows API.
-
Comments that are just tautologies are not very useful
-
-
Why are you using
-
You have some redundant casts, like in
-
Why does
-
The "Fuck you Windows" comment is unprofessional and unlikely to impress any readers of your code. I (and future readers) will also have no idea what the purpose of the comment is, so it's useless altogether
-
Comments that are just tautologies are not very useful
-
typedef struct is a pointless idiom in C++, as C++ doesn't have tag names-
Why are you using
unsigned short (2 bytes wide) when the DEVMODE structure uses DWORD (32-bits wide)? In general, you should use the Windows typedef to stay consistent and avoid unnecessary type issues-
You have some redundant casts, like in
tsGetMonitorCount(). Also C-style casts can sometimes silence warnings which is not what you want-
Why does
tsGetVideoMode return a bool? I would expect it to return a video mode-
The "Fuck you Windows" comment is unprofessional and unlikely to impress any readers of your code. I (and future readers) will also have no idea what the purpose of the comment is, so it's useless altogether
Context
StackExchange Code Review Q#128998, answer score: 4
Revisions (0)
No revisions yet.