patterncppMinor
C++ code to launch a Windows 8 app
Viewed 0 times
codelaunchwindowsapp
Problem
This is my first C++ in many years. It's based on some stuff I found on the internet, including a Microsoft article on a similar topic. But I'm sure there are C++ idioms that might make this better or leaner (my main goal).
Pretty small and simple, but I want to open-source this. And I definitely know the feeling of looking at JavaScript newbies' code and going "ugh! why didn't they just do !" So I want to come across as a suave sophisticated C++ programmer instead ;)
#include
#include
#include
int wmain(int argc, wchar_t* argv[])
{
HRESULT hr = S_OK;
if (argc aam = nullptr;
hr = CoCreateInstance(CLSID_ApplicationActivationManager, nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&aam));
if (FAILED(hr))
{
wprintf(L"Error creating ApplicationActivationManager");
CoUninitialize();
return hr;
}
hr = CoAllowSetForegroundWindow(aam, nullptr);
if (FAILED(hr))
{
wprintf(L"Error calling CoAllowSetForegroundWindow");
CoUninitialize();
return hr;
}
unsigned long pid = 0;
hr = aam->ActivateApplication(appId, nullptr, AO_NONE, &pid);
if (FAILED(hr))
{
wprintf(L"Error calling ActivateApplication");
CoUninitialize();
return hr;
}
CoUninitialize();
return 0;
}Pretty small and simple, but I want to open-source this. And I definitely know the feeling of looking at JavaScript newbies' code and going "ugh! why didn't they just do !" So I want to come across as a suave sophisticated C++ programmer instead ;)
Solution
One piece of feedback is that your error handling code is very repetitive. You are calling
There are a few ways around this that I've seen.
As an example of how you might do that in COM code, here's the "wrong" way:
There are several choices for the "right" way. One of them would be:
If you are not religiously opposed to
Example:
With this approach you can have simply:
Then after this block, you can
(Notice I didn't initialize COM in a constructor. This allows us to inspect the
CoUninitialize in every failure block. That might seem OK for one cleanup task (though I'd tend to disagree), but once you have N things to clean up on failure (or even successful return), it gets to be a lot of effort and maintenance.There are a few ways around this that I've seen.
- Have a single block that cleans up (frees any buffers, unintializes COM, whatever). That goes at the end if your function. Make all failure and success paths reach this block.
As an example of how you might do that in COM code, here's the "wrong" way:
hr = Foo();
if (FAILED(hr))
{
Cleanup();
}
hr = Bar();
if (FAILED(hr))
{
Cleanup();
}
Cleanup();There are several choices for the "right" way. One of them would be:
if (SUCCEEDED(hr))
hr = Foo();
if (SUCCEEDED(hr))
hr = Bar();
Cleanup();If you are not religiously opposed to
goto (this probably makes more sense in C than C++), this approach is also common:hr = Foo();
if (FAILED(hr))
goto cleanup;
hr = Bar();
if (FAILED(hr))
goto cleanup;
cleanup:
Cleanup();- There are of course more C++ like (rather than C style) ways to prevent cleanup from becoming too much of a hassle. In particular it would probably be a better idea to wrap initialization and de-initialization in the RAII idiom. In this example (COM initialization), you might have a class that (1) initializes COM, (2) tracks that it's been initialized, and (3) in its destructor, deinitializes COM if it has been initialized. Something like this: [nb: I am typing in a web form, it may not be perfect :-)]
Example:
class ComInitialization
{
bool init;
public:
ComInitialization() : init(false) {}
HRESULT Initialize()
{
HRESULT hr = S_OK;
if (!init)
{
hr = CoInitializeEx(/* ... */);
if (SUCCEEDED(hr))
{
init = true;
}
}
return hr;
}
~ComInitialization()
{
if (init)
{
CoUninitialize();
}
}
}With this approach you can have simply:
ComInitialization comInit;
hr = comInit.Initialize();Then after this block, you can
return in any place you like, success or failure, even throw exceptions, and COM will still get uninitialized.(Notice I didn't initialize COM in a constructor. This allows us to inspect the
HRESULT on failure without wrapping it in an exception. I'm sure many would suggest wrapping HRESULTs in exceptions. This is not personally my taste. YMMV.)Code Snippets
hr = Foo();
if (FAILED(hr))
{
Cleanup();
}
hr = Bar();
if (FAILED(hr))
{
Cleanup();
}
Cleanup();if (SUCCEEDED(hr))
hr = Foo();
if (SUCCEEDED(hr))
hr = Bar();
Cleanup();hr = Foo();
if (FAILED(hr))
goto cleanup;
hr = Bar();
if (FAILED(hr))
goto cleanup;
cleanup:
Cleanup();class ComInitialization
{
bool init;
public:
ComInitialization() : init(false) {}
HRESULT Initialize()
{
HRESULT hr = S_OK;
if (!init)
{
hr = CoInitializeEx(/* ... */);
if (SUCCEEDED(hr))
{
init = true;
}
}
return hr;
}
~ComInitialization()
{
if (init)
{
CoUninitialize();
}
}
}ComInitialization comInit;
hr = comInit.Initialize();Context
StackExchange Code Review Q#19080, answer score: 4
Revisions (0)
No revisions yet.