Testing if TreeView's node is checked

I have recently answered a question on Stack Overflow where OP wanted to programmatically determine if TreeView's node is checked or not, among other things. My answer got accepted since everything seems to work fine.

Still, after reading MSDN documentation for TVN_KEYDOWN, I found the part in the "Return value" section that mentioned incremental search. Therefore I have decided to ask what is a proper return value for my usage of that notification and have concluded that I should return a nonzero value.

Question:

My concern is if my code is the best way for testing if the tree's node is checked or not. That is why I came here to ask for review of that code, and for possible suggestions for improving it.

Here is the tree's creation in WM_CREATE handler:

```
case WM_CREATE:
{
// this is your treeview

TreeView = CreateWindowEx(0, WC_TREEVIEW,
TEXT("Tree View"), WS_VISIBLE | WS_CHILD,
0, 0, 200, 500,
hwnd, (HMENU)ID_TREE_VIEW, GetModuleHandle(NULL), NULL);

/ enable checkboxes **/

DWORD dwStyle = GetWindowLong( TreeView , GWL_STYLE);
dwStyle |= TVS_CHECKBOXES;
SetWindowLongPtr( TreeView , GWL_STYLE, dwStyle );

/ add items and subitems ****/

// add root item

TVINSERTSTRUCT tvis = {0};

tvis.item.mask = TVIF_TEXT;
tvis.item.pszText = L"This is root item";
tvis.hInsertAfter = TVI_LAST;
tvis.hParent = TVI_ROOT;

HTREEITEM hRootItem = reinterpret_cast( SendMessage( TreeView ,
TVM_INSERTITEM, 0, reinterpret_cast( &tvis ) ) );

// add firts subitem for the hTreeItem

memset( &tvis, 0, sizeof(TVINSERTSTRUCT) );

tvis.item.mask = TVIF_TEXT;
tvis.item.pszText = L"This is first subitem";
tvis.hInsertAfter = TVI_LAST;
tvis.hParent = hRootItem;

HTREEITEM hTreeSubItem1 = reinterpret_cast( SendMessage( Tree

I have learned more about tree view check-boxes than I wanted to ;)

From a first glance I only have 2 observations:

• You have 10 lines of copy pasted code in there, you should have a function for that

-
This looks terrible:

// Return zero if it's not checked, or nonzero otherwise.
if( (BOOL)(tvItem.state >> 12) - 1 )

• Why 12 -> Magic constant ?

But then, from reading this and that, it seems that is the right way of doing it. For the sanity of whoever would maintain this, you need a lengthy comment block there because it just looks wrong. The comment should mention why you need to shift 12 positions, and that the index can be 1 or 2 ( so that is why you subtract 1 ).

I find it funny that there is a #define INDEXTOSTATEIMAGEMASK(i) ((i) > 12), perhaps you could create this macro yourself and make the code more readable by employing that macro.

As an aside, there is a 'cleaner' way of getting the state, but that works only as of Vista, so I would stick to your approach.

StackExchange Code Review Q#44798, answer score: 4