CodeSOD: A Promise of Timing

Asynchronous programming is hard, and there's never been a perfect way to solve that problem. One of the most widely used solutions is the "promise" or "future". We wrap the asynchronous process up in a, well, promise of a future result. "Someday, there will be data here, I hope." The real beauty of promises comes from their composability- "getData promises to fetch some records, and then the calling method can promise to display them."

Of course, it's still asynchronous, and when an application has multiple asynchronous processes happening at the same time, "weird behavior" can happen, thanks to timing issues. Keith W encountered one of those timing related Heisenbugs, and became immediately suspicious about how it was getting invoked:

this.disableUI("Loading form...");this.init(workflowId, ordinal).done(() => this.enableUI("Form loaded!"));

init isn't passing data to the next promise in the chain. So what actually happens in the init method?

public init(workflowId: string, ordinal: number): JQueryPromise<any> { var self = this; var promise = $.Deferred<boolean>(); $.when( $.ajax({ type: "GET", url: getEndpoint("work", "GetFormInputForm"), data: { id: workflowId, ordinal: ordinal }, contentType: "application/json", success(result) { if (result) { var formType = result.FormType || Forms.FormType.Standard; self.formMetaDataExcludingValues = result; var customForm = new Forms.CustomForm(formType, Forms.FormEditorMode.DataInput, result); self.CustomForm(customForm); self.flatControlList = customForm.flattenControls(); ko.utils.arrayForEach(self.flatControlList, (cnt) => { var row = { name: cnt.name(), value: {}, displayText: "", required: cnt.required() }; if (cnt instanceof Forms.SelectControl) { row.value = cnt.values(); } else if (cnt instanceof Forms.ContactControl || cnt instanceof Forms.OrganisationControl) { row.displayText = cnt.displayName(); row.value = cnt.value(); } else if (cnt instanceof Forms.SequenceControl) { row.value = ""; } else if (cnt instanceof Forms.AssetControl) { row.value = cnt.value(); row.displayText = row.value ? cnt.displayName() : null; } else if (cnt instanceof Forms.CategoryControl) { row.value = cnt.cachedValue; } else { row.value = cnt.value(); } self.defaultValues.push(new ControlValue(row)); }); } } }) ).done(() => { $.ajax({ type: "GET", url: url, data: { id: workflowId, ordinal: ordinal, numberOfInstances: self.NoOfInstances() }, contentType: "application/json", success(result) { if (result) { var formType = result.FormType || Forms.FormType.Standard; if (!multiple) { self.CurrentInstanceNo(1); // store a blank form control values self.saveCurrentFormValues().done(() => self.applyKoBinding().done(() => { console.info("Data Saved Once."); })); } else { self.Forms([]); self.CurrentInstanceNo(self.NoOfInstances()); self.saveAllFormValues(result).done(() => self.applyKoBinding().done(() => { console.info("Data Saved for all."); })); } } } }) }) promise.resolve(true); return promise;}

This code is awful at first glance. It's complicated spaghetti code, with all sorts of stateful logic that manipulates properties of the owning object. That's your impression at first glance, but it's actually worse.

Let's highlight the important parts:

public init(workflowId: string, ordinal: number): JQueryPromise<any> { var promise = $.Deferred<boolean>(); //big pile of asynchronous garbage promise.resolve(true); return promise;}

Yes, this init method returns a promise, but that promise isn't contingent on any other promises being made, here. promise.resolve(true) essentially renders this promise synchronous, not asynchronous. Instead of waiting for all the pending requests to complete, it immediately claims to have succeeded and fulfilled its promise.

No wonder there's a"
"
"
"
"
"
"
"
" timing issue.

[Advertisement] Release!is a light card game about software and the people who make it. Play with 2-5 people, or up to 10 with two copies - only $9.95 shipped!