Opened 10 years ago

Closed 9 years ago

#9494 closed enhancement (wontfix)

Generalize the map in dojo.string.substitute

Reported by: Eugene Lazutkin Owned by: Eugene Lazutkin
Priority: low Milestone: 1.6
Component: Core Version: 1.3.0
Keywords: Cc: dante, alex, Adam Peller, James Burke
Blocked By: Blocking:

Description

The 2nd argument of dojo.string.substitute is an object or array, which is used as a dictionary to look up values using keys found in the template. Allowing it to be a mapping function makes the whole templating process more general, e.g. it allows complex lookups, on-demand lazy calculations, and so on.

Attachments (1)

dojo-string-substitute.diff (2.8 KB) - added by Eugene Lazutkin 10 years ago.

Download all attachments as: .zip

Change History (24)

Changed 10 years ago by Eugene Lazutkin

Attachment: dojo-string-substitute.diff added

comment:1 Changed 10 years ago by Eugene Lazutkin

Cc: dante alex Eugene Lazutkin added
Owner: changed from anonymous to James Burke

The attached patch includes the code itself, the inline documentation, and the unit test. The example reflects real-life requirements I encountered in the wild. The implementation was tested on real projects.

The implementation is simple, relatively low-risk, touches the Core not the Base. In the default case the overhead is one extra function wrapper, which is not big considering that it wraps dojo.getObject().

comment:2 Changed 10 years ago by bill

It seems good. Maybe we want to remove the format and transform features for 2.0 as users can get the same effect through making the second argument a function.

comment:3 Changed 10 years ago by Eugene Lazutkin

I think format is a good idea:

  • It works well with maps and mapping functions alike.
  • The same result can be formatted differently --- it is a common enough use case.
  • It doesn't cost much performance-wise and implementation-wise.

The only problem with format as I see it is that dojo.string.substitute reuses the same thisObject for everything including looking for format and transform. In my example I used it as a data source => it'll be unnatural to put formatters in it.

But transform looks excessive to me too. I never had a use case for it, especially with the mapping function in place.

One possible solution for Dojo 2.0 is to have a signature like that (pseudo-code):

substitute = function(
  String template,
  Object|Array|Function map,
  Object? formatters
);

In this case template is the same as it is now, map can be an object/array (like now) or a mapping function (like proposed), transform is gone, thisObject is gone, formatters is a lookup dictionary for formatters (otherwise dojo.global is used).

Why no thisObject in this schema? It can be easily replaced with proper dojo.hitch(). My example re-written (pseudo-code):

dojo.string.substitute(
  "${count} payments averaging $${avg:withCommas} per payment.",
  dojo.hitch({ payments: [11, 16, 12] },
    function(key){
      switch(key){
        case "count": return this.payments.length;
        case "min":   return Math.min.apply(Math, this.payments);
        case "max":   return Math.max.apply(Math, this.payments);
        case "sum":   return sum(this.payments);
        case "avg":   return sum(this.payments) / this.payments.length;
      }
    }
  },
  {
    withCommas: function(x){...},
    redWhenNegative: function(x){...}
  }
);

Another way to re-write the same example using the same API, but pulling data from the closure:

var payments = [11, 16, 12];
dojo.string.substitute(
  "${count} payments averaging $${avg:withCommas} per payment.",
  function(key){
    switch(key){
      case "count": return payments.length;
      case "min":   return Math.min.apply(Math, payments);
      case "max":   return Math.max.apply(Math, payments);
      case "sum":   return sum(payments);
      case "avg":   return sum(payments) / payments.length;
    }
  },
  {
    withCommas: function(x){...},
    redWhenNegative: function(x){...}
  }
);

Not that big of a difference, and both patterns should be familiar to most Dojo users.

On a superficial level we can just remove transform from the signature and use thisObject as formatters --- logically it is the same when transform is out of the picture.

Hmm, come to think of it, we can implement the mapping function argument like in my Dojo 2.0 example --- with pure function (use explicit dojo.hitch() to bind it with an object). Heh, the future is now! The only difference is the necessity of null between map and thisObject to indicate that transform is not used.

If we have enough supporters of the latter idea, I can rework the patch.

comment:4 Changed 10 years ago by Adam Peller

Cc: Adam Peller added

comment:5 Changed 10 years ago by Adam Peller

Sounds like a great enhancement, and perhaps thisObject is no longer needed, I'm not sure. A couple things to consider about the other params: format is nice because it's specified in the data, not in the code. You say ${amount:withCommas} so the template writer actually gets to determine this, as opposed to embedding the logic in the application that reads it. As for transform, note that the transform is performed on the substitution data before it is substituted. The most common scenario is where you know you're inserting a string into HTML. Your template may already have the appropriate escapes, but the data you're substituting is coming straight from some data source, so you want to do &->& before doing the substitution. Perhaps we should just expect the result to be at the same level and the caller can transform the whole thing, but it seemed that we needed this for dijit.

comment:6 Changed 10 years ago by bill

format is nice because it's specified in the data, not in the code

I'm sure formatting is useful; just look at how often it's used in printf(). I just wonder if it should [optionally] be handled by the map function, like:

dojo.string.substitute(
  "${count} payments averaging ${avg:0.00} per payment.",
  function(key, format){ ... }
);

That would call mapFunction("avg", "0.00"). Note that this is nice because it supports formats like 0.00 that don't map to function names.

transform is performed on the substitution data before it is substituted

The idea of doing a transform is also useful, but again it can be done by the map function. Even if you have a name --> value hash for a map, you just need to write a little inline function:

dojo.string.substitute(
  "${foo} payments averaging ${bar} per payment.",
   function(key){ return myTransform(myMap[key]); }
);

Anyway, I'm OK w/anything but just food for thought.

comment:7 Changed 10 years ago by Eugene Lazutkin

Cc: James Burke added; Eugene Lazutkin removed
Owner: changed from James Burke to Eugene Lazutkin
Status: newassigned

I agree with bill that transforms can be trivially implemented with a mapping function in place.

Regarding format as a second argument to the mapping function: if mapping function receives the whole key, it can split it itself. In fact it is a way to implement custom parameters. Something like that:

dojo.string.substitute(
  "${count} payments averaging ${avg:0.00} per payment.",
  function(name){
    var parts = name.split(":"),
        key = parts[0],
        format = parts[1];
    ...
  }
);

Obviously format parameters can be more complicated.

Another way to implement formatters is (again) to use a mapping function:

function subWithFormats(tmpl, map, formatters){
  var mapFunc = dojo.isFunction(map) ? map : function(k){ return map[k]; };
  return dojo.string.substitute(tmpl, function(name){
    var parts = name.split(":"),
        value = mapFunc(parts[0]);
    if(parts.length > 1 && formatters && formatters[parts[1]]){
      value = formatters[parts[1]](value, parts.slice(2));
    }
    return value;
  });
}

Example of a formatter:

function fixed(value, opt){
  if(opt && opt.length){
    return value.toFixed(parseInt(opt[0]));
  }
  return value.toFixed();
}

Usage:

subWithFormats("The price is ${price:fixed:2} USD", {price: 2.75}, {fixed: fixed});

It can be implemented as a convenience function, if enough people see its utility.

comment:8 Changed 10 years ago by Eugene Lazutkin

Actually for my needs a wrote a mini version too. Now I think it may be of use:

var d = dojo;

d.sub = function(tmpl, map, ctx){
        ctx = ctx || d.global;
        var f = d.isFunction(map) || typeof map == "string" ?
                d.hitch(ctx, map) :
                function(k){ return d.getObject(k, false, map); };
        return tmpl.replace(/\$\{([^\}]+)\}/g,
                function(_, k){ return f(k).toString(); });
};

It is small enough to fit in the core (lang.js?), and flexible enough to (reimplement) dojo.string.substitute. I suspect it is faster too.

Thoughts?

comment:9 Changed 10 years ago by Eugene Lazutkin

BTW, the context (ctx) can be removed making code even shorter. I used it to pass an object I want to use as the underlying map. If it is removed, we don't need to check if map is a string, we don't need dojo.hitch() call, and no need to supply the default value for a context.

So the second version is:

var d = dojo;

d.sub = function(tmpl, map){
        var f = d.isFunction(map) ? map :
                function(k){ return d.getObject(k, false, map); };
        return tmpl.replace(/\$\{([^\}]+)\}/g,
                function(_, k){ return f(k).toString(); });
};

Both versions support the same template syntax as dojo.string.substitute() sans formatters, but mapping functions are free to implement them.

Any preferences?

comment:10 Changed 10 years ago by bill

They both look good; I don't have a strong preference on whether or not we add a small function like this to base.

Should dojo.sub() take a context parameter? I'm not sure. I thought about whether or not most dojo methods takes a context. dojo.forEach() and dojo.connect() take a ctx whereas dojo.xhr() does not... it seems random although maybe there's something I'm missing.

Probably if dojo.sub() is added to base then dojo.string.substitute() should be modified to call it.

comment:11 Changed 10 years ago by Eugene Lazutkin

The final candidate is essentially a one-liner:

var d = dojo, _pattern = /\$\{([^\}]+)\}/g;

d.sub = function(tmpl, map, pattern){
  return tmpl.replace(pattern || _pattern, d.isFunction(map) ?
    map : function(_, k){ return d.getObject(k, false, map); });
};

The 1st parameter is a string to be used for substitutions. The 2nd is a function or a dictionary. The 3rd is an optional pattern to be replaced --- sometimes ${} clashes with server-side variables and should be changed, some people prefer {}, and so on.

I struggled with the optional context argument for a function, and decided against it. While it is possible to fit it in the signature without any clashes, it is hard to document, and can be confusing for users.

I plan to add it to dojo/_base/lang.js next to dojo.trim().

Let me know if you spot any problems with the final version. Otherwise I'll commit it this weekend.

comment:12 Changed 10 years ago by James Burke

I am a bit concerned about the name, dojo.sub -- that is shortcut I have also used for a dojo.subscribe variant, that I believe I took from plugd? dojo.replace would be an option. dojo.template(), dojo.s(), dojo.str()?

I like that it does not automatically create a hitched function like dojo.string.substitute().

comment:13 Changed 10 years ago by Eugene Lazutkin

I see what you mean. Technically speaking "sub" is a verb, but the only meaning is "to act as a substitute" --- not exactly what this function does.

I think that dojo.template is probably the most explicit name, yet it is not a verb. Out of all proposed names the only one is a distinct verb: dojo.replace. Still I don't feel that it reflects what it does with a simple "replace", but the only other possible name I can think of involves the camel case: dojo.fillIn? Then again dojo.replace echoes String.replace, and can be thought of as a specialized version of it, which is the case.

Most probably we need more ideas/votes.

comment:14 Changed 10 years ago by Eugene Lazutkin

Milestone: tbd1.4

comment:15 Changed 10 years ago by bill

Milestone: 1.41.5

... since we passed the 1.4 beta cutoff

comment:16 Changed 10 years ago by Eugene Lazutkin

(In [20666]) Committing dojo.replace(), !strict, refs #9494.

comment:17 Changed 10 years ago by Eugene Lazutkin

Forgot to commit dojo.replace() before. The commit includes the one liner in question, inline docs, and examples, as well as unit tests.

One difference with the discussed implementation is that by default it matches patterns like {xxx} instead of ${xxx} --- the latter proved to be a nuisance when working with server-side frameworks like Struts, which try to substitute such patterns server-side. In any case users are free to redefine the pattern --- it is exposed as an argument.

To minimize potential impact dojo.string.substitute() will be converted to this facility later post 1.4 release.

comment:18 Changed 10 years ago by dante

Milestone: 1.51.4

argh. Please add docs to http://docs.dojocampus.org/dojo/replace and http://docs.dojocampus.org/dojo/index and http://docs.dojocampus.org/releasenotes/1.4 so people know this new [late addition] api exists.

comment:19 Changed 10 years ago by dante

it also isn't showing up in the api docs http://api.dojotoolkit.org/jsdoc/HEAD/dojo.replace .. will need to work with neil to see why.

comment:20 in reply to:  18 Changed 10 years ago by Eugene Lazutkin

Replying to dante:

argh. Please add docs to http://docs.dojocampus.org/dojo/replace and http://docs.dojocampus.org/dojo/index and http://docs.dojocampus.org/releasenotes/1.4 so people know this new [late addition] api exists.

Done.

comment:21 Changed 10 years ago by Eugene Lazutkin

Milestone: 1.41.5

bumping tickets that didn't make the 1.4 cut, but most likely to go in the next point release.

comment:22 Changed 9 years ago by Eugene Lazutkin

Milestone: 1.51.6

comment:23 Changed 9 years ago by Eugene Lazutkin

Resolution: wontfix
Status: assignedclosed

Looks fine for me. Let's postpone the possible template revision until 2.0.

Note: See TracTickets for help on using tickets.