Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#16407 closed enhancement (patchwelcome)

string.substitute should handle missing replacement values better

Reported by: Mangala Sadhu Sangeet Singh Khalsa Owned by: Adam Peller
Priority: undecided Milestone: tbd
Component: String Version: 1.8.1
Keywords: Cc:
Blocked By: Blocking:

Description

string.substitute(template, map) - if any keys from template are missing in map, the method throws:

"Error: Cannot call method 'toString' of undefined".

At a minimum, it should throw an error with an informative, relevant message.

It may be good to simply ignore keys with no matching replacement value, or implement behavior definable by the caller:

  • throw error
  • ignore
  • replace with supplied value

Change History (6)

comment:1 Changed 7 years ago by Adam Peller

It was an intentional choice to fail on missing keys, also to keep the method extremely minimal. Dojo typically does not do bounds checking or issue error messages. It is unfortunate that the resulting error is cryptic. It is possible to use a function to transform the values. Perhaps that could be used to provide the behavior you seek.

Last edited 7 years ago by Adam Peller (previous) (diff)

comment:2 Changed 7 years ago by Mangala Sadhu Sangeet Singh Khalsa

Dojo is the foundational code for building applications - it's a very bad choice to not have it be robust. When you use a library, you want it to work, or tell you why it didn't. The consumer of a library should not be required to dig into the library's source every time something goes wrong. I realize this attitude is pervasive in Dojo, and believe it is detrimental to the usability and popularity of Dojo.

comment:3 Changed 7 years ago by Adam Peller

Failing silently when keys are missing does not fit some people's definition of robust. Adding error checking for arguments in Dojo is considered bloat. If you'd like to contribute documentation to help correct what you see as a confusing situation, contributions are welcome.

comment:4 Changed 7 years ago by Adam Peller

Resolution: patchwelcome
Status: newclosed

comment:5 Changed 7 years ago by Adam Peller

as I recall, the only reason the .toString() is there is to cause an exception for undefined values.

comment:6 Changed 7 years ago by cgaeking

Why not providing a simple method to check a map against a template. I personally use something like:

content = string.substitute(content, getFilledSubstituteMap(content, myResources));

function getFilledSubstituteMap(content, map) {
    var varname="";
    var variable=false;
    for (var i = 0; i < content.length; i++) {
    	var n=content.charAt(i);
        if (variable==false) {
            if (n=="$" && i<content.length && content.charAt(i+1)=="{") {
                variable=true;
            }
        } else {
            if (n=="}") {
                var found=false;
                for (var langvar in map) {
                    if (varname==langvar) {
                        found=true;
                        break;
                    }
                }
                if (!found) map[varname]="???"+varname+"???";
                varname="";
                variable=false;
            } else if (n!="{") {
                varname=varname+n;
            }
        }
    }
    return map;
}

This adds unresolved variables to the resulting template with ???variable??? This is the same behaviour like when using <fmt:message key... in JSTL. It is very usefull when developing. Like this templates can be developed easily without updating the language files constantly and having errors in the logs.

So why not considering something like this as a seperate/additional method? If one has finished developing templates and language files the method is not needed anymore anyway

Note: See TracTickets for help on using tickets.