CodeSOD: Break my Validation

Linda inherited an inner-platform front-end framework. It was the kind of UI framework with an average file size of 1,000 lines of code, and an average test coverage of 0%.

Like most UI frameworks, it had a system for doing client side validation. Like most inner-platform UI frameworks, the validation system was fragile, confusing, and impossible to understand.

This code illustrates some of the problems:

/**
 * Modify a validator key, e.g change minValue or disable required
 *
 * @param fieldName
 * @param validatorKey - of the specific validator
 * @param key - the key to change
 * @param value - the value to set
 */
modifyValidatorValue: function (fieldName, validatorKey, key, value) {

	if (!helper.isNullOrUndefined(fieldName)) {
		// Iterate over fields
		for (var i in this.fields) {
			if (this.fields.hasOwnProperty(i)) {
				var field = this.fields[i];
				if (field.name === fieldName) {
					if (!helper.isNullOrUndefined(validatorKey)) {
						if (field.hasOwnProperty('validators')) {
							// Iterate over validators
							for (var j in field.validators) {
								if (field.validators.hasOwnProperty(j)) {
									var validator = field.validators[j];
									if (validator.key === validatorKey) {
										if (!helper.isNullOrUndefined(key) && !helper.isNullOrUndefined(value)) {
											if (validator.hasOwnProperty(key)) {
												validator[key] = value;
											}
										}
										break;
									}
								}
							}
						}
					}
					break;
				}
			}
		}
	}

}

What this code needs to do is find the field for a given name, check the list of validators for that field, and update a value on that validator.

Normally, in JavaScript, you’d do this by using an object/dictionary and accessing things directly by their keys. This, instead, iterates across all the fields on the object and all the validators on that field.

It’s smart code, though, as the developer knew that once they found the fields in question, they could exit the loops, so they added a few breaks to exit. I think those breaks are in the right place. To be sure, I’d need to count curly braces, and I’m worried that I don’t have enough fingers and toes to count them all in this case.

According to the git log, this code was added in exactly this form, and hasn’t been touched since.

hljs.initHighlightingOnLoad();

[Advertisement]
Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!


Source: The Daily WTF