dojox/analytics/consoleMessages plugin missing "var"

[mm]

define(["dojo/_base/lang","../_base", "dojo/_base/config", "dojo/aspect"
], function(lang, dxa, config, aspect){
	/*=====
		dxa = dojox.analytics;
		aspect = dojo.aspect;
	=====*/
	consoleMessages = lang.getObject("dojox.analytics.plugins.consoleMessages", true);

		// summary:
		//	plugin to have analyitcs return the base info dojo collects
		this.addData = lang.hitch(dxa, "addData", "consoleMessages");

		var lvls = config["consoleLogFuncs"] || ["error", "warn", "info", "rlog"];
		if(!console){
			console = {};
		}

		for(var i = 0; i < lvls.length; i++){
			if(console[lvls[i]]){
				aspect.after(console, lvls[i], lang.hitch(this, "addData", lvls[i]),true);
			}else{
				console[lvls[i]] = lang.hitch(this, "addData", lvls[i]);
			}
		}
	return consoleMessages;
});

Several questions for this source code:

Missing var in front of consoleMessages (global variable ? Purpose ? I hope not)

Using lang.getObject(, true) instead of declare() (purpose ? Avoiding dependency on base/_declare and preferring lax style instead ?)

Repeated lvls[i] lookup and lengthy code ?

[mm]

proposed rewrite (comments welcomed)

define(["dojo/_base/lang","../_base", "dojo/_base/config", "dojo/aspect"
], function(lang, dxa, config, aspect){
	/*=====
		dxa = dojox.analytics;
		aspect = dojo.aspect;
	=====*/
	var consoleMessages = lang.getObject("dojox.analytics.plugins.consoleMessages", true);

		// summary:
		//	plugin to have analyitcs return the base info dojo collects
		this.addData = lang.hitch(dxa, "addData", "consoleMessages");

		var lvls = config["consoleLogFuncs"] || ["error", "warn", "info", "rlog"];
		console || (console={});

		for(var i = 0; i < lvls.length; i++){
			var fnName=lvls[i], _addData=lang.hitch(this, "addData", fnName);
			if(console[fnName]){
				aspect.after(console, fnName, _addData , true);
			}else{
				console[fnName] = _addData;
			}
		}
	return consoleMessages;
});

[bill]

mm, I'm not sure who you are? We can't take code unless you've filed a ​CLA, although we can add the missing "var" that you mentioned.

As for lang.getObject("dojox.analytics.plugins.consoleMessages", true), that's a typical way to create a global variable, but will disappear in 2.0 when we get rid of global variables, and will be replaced by something like

var consoleMessages = {};

[mm]

my CLA is filled months ago. Please check your lists.

[bill]

I checked the lists and there is no one named "mm". What name are you listed under?

[mm]

Martin Marko ([email protected]…) Thanx

[mm]

status ? please ?

[dylan]

In [28772]:

fixes #14955, thanks mm

[dylan]

I've accepted most of this patch, but not the change for

console

(console={});

Mostly because our post-commit hook doesn't like it, and I didn't want to override it for a change that is minor.