HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Monitor and video mode querying

Submitted by: @import:stackexchange-codereview··
0
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;

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

-
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.