In the previous post, we discussed using WIL to tame COM APIs. At the end, we had translated a Time Trigger sample from the Task Scheduler API from a very C-like structure into something approaching modern C++. Still, that example has dense code blocks with large banner comments trying to explain what is going on, e.g.:
// ------------------------------------------------------ // Get the trigger collection to insert the time trigger. wil::com_ptr<ITriggerCollection> pTriggerCollection; THROW_IF_FAILED_MSG(pTask->get_Triggers(pTriggerCollection.put()), "Cannot get trigger collection"); // Add the time trigger to the task. wil::com_ptr<ITrigger> pTrigger; THROW_IF_FAILED_MSG(pTriggerCollection->Create(TASK_TRIGGER_TIME, pTrigger.put()), "Cannot create trigger"); auto pTimeTrigger = pTrigger.query<ITimeTrigger>(); THROW_IF_FAILED_MSG(pTimeTrigger->put_Id(_bstr_t(L"Trigger1")), "Cannot put trigger ID"); THROW_IF_FAILED_MSG(pTimeTrigger->put_EndBoundary(_bstr_t(L"2015-05-02T08:00:00")), "Cannot put end boundary on trigger");
I don’t want to get too far into a debate about self-documenting code. But clearly, this code is less clear than it could be because of all the details in your face. The comments are intending to help visually break up the sections into smaller subsections that you can reason about — procedures, if you will!
Now of course, a sample program should not attempt to abstract away all the details. When you are reading a sample, you usually do care about the steps you have to perform to get the job done. Hiding those steps would actually be counterproductive for the intended audience, those who are there to learn and understand the API and its specifics. However, that is an altogether different goal than we would have as consumers of this API in a larger system. All useful software has dependencies. But we do not need to make those dependencies front and center in our design.
Let’s do an exercise in procedural decomposition to help clarify the workflow we have in our sample. Recall that we ended up with this general structure:
void run() { // . . . do everything in here! } int main() try { run(); printf("\n Success! Task successfully registered. "); } CATCH_RETURN()
So we have managed to decompose the problem space into two steps, run and finish. We can probably do better than that. Let’s extract every smaller step into its own free function to see that where that leads:
- extract init_wil: setting up the error log behavior isn’t really part of the main flow, so let’s call that from another function inside main itself
- extract init_com: of course, the first real step is initializing COM; we’ll make sure to return and hold the RAII wrapper for the entire scope of the
run
function - extract get_executable_path: hide the details, we just care about the resulting string
- extract connect_task_service: here we can take out both the initialization and the Connect call since it’s of no use to do one without the other
- extract get_root_folder
- remove unnecessary DeleteTask call: we already specify
TASK_CREATE_OR_UPDATE
when we register the task at the end, so this is superfluous - extract create_task
- extract set_author
- extract set_logon_type
- extract set_settings: and while we’re at it, use std::chrono::duration types to represent the timeout
- extract add_time_trigger: and while we’re at it, use std::chrono::time_point convenience types to represent date-time values
- extract add_exec_action
- extract save: decomposition complete!
- Remove stdio.h dependency: this printf bothers me, switch to cout
- Remove comutil.h dependencies: let’s get rid of the legacy
_bstr_t
and replace with WIL unique_bstr
Our new program is 186 lines, slightly longer than before, but the code in the core main/run flow looks like this:
void run() { auto cleanup = init_com(); auto pService = connect_task_service(); auto pRootFolder = get_root_folder(*pService); auto pTask = create_task(*pService); set_author(*pTask, L"Author Name"); set_logon_type(*pTask, TASK_LOGON_INTERACTIVE_TOKEN); set_settings(*pTask, true, std::chrono::minutes(5)); add_time_trigger(*pTask, L"Trigger1", make_date_time(2005y / 1 / 1, 12h + 5min), make_date_time(2015y / 5 / 2, 8h)); add_exec_action(*pTask, get_executable_path()); save(*pTask, L"Time Trigger Test Task", *pRootFolder); } int main() try { init_wil(); run(); std::cout << "Success! Task successfully registered.\n"; } CATCH_RETURN()
For procedural style you can’t do much better. The code is clear and concise, and the details mostly out of view. We could stop here but we are missing one key reason to favor C++ over C — encapsulation! The fact that nearly all of these functions require a special first parameter to “do” the action means that we have a class somewhere deep down that wants to sprout. Recall that a class method in pretty much every OO language has a this
or self
parameter (almost always) hidden inside. Let’s do one final pass and make some classes:
- extract TaskService class: use a static named constructor to hide the COM details
- extract Task class: move most of the free functions into this class
- extract TaskFolder class: move the
save
method here, and acceptTask
as a parameter
We paid a premium in code lines to add these classes (our sample is 224 lines now), but we gained some domain modeling clarity:
void run() { auto cleanup = init_com(); auto service = TaskService::connect(); auto folder = service.get_root_folder(); auto task = service.create_task(); task.set_author(L"Author Name"); task.set_logon_type(TASK_LOGON_INTERACTIVE_TOKEN); task.set_settings(true, std::chrono::minutes(5)); task.add_time_trigger(L"Trigger1", make_date_time(2005y / 1 / 1, 12h + 5min), make_date_time(2015y / 5 / 2, 8h)); task.add_exec_action(get_executable_path()); folder.save(task, L"Time Trigger Test Task"); }
This is a great start! But all of our code is in main
and we have largely ignored the fact that nothing here is tested, or testable, for that matter. Don’t worry, we’ll surely revisit this and fix it.
Pingback: Stay COM: stubs and testing – WriteAsync .NET