Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

A friend of mine has a popular open source JavaScript library which is quite well used in the community. He is now going through a process of refactoring and applying best practices and conventions and is wanting to upper case all constructor functions. So in the past where he may have defined a chart object, he now wants it to be called Chart to stick with convention.

Because the library is well used, he would rather warn devs when they use the old function names and eventually deprecate them.

I've recommended using a helper function :

var ObsoleteWithReplacement = function(replacementFunction, oldFnName, newFnName) {
    var wrapper = function() {
       console.warn("WARNING! Obsolete function called.  Function '" + oldFnName + "' has been deprecated, please use the new '" + newFnName + "' function instead!");

        replacementFunction.apply(this, arguments);
    }
    wrapper.prototype = replacementFunction.prototype;

    return wrapper;
}

Which could be called as follows : http://jsfiddle.net/rLNep/1/

Is this a good way to go about it? Can you recommend a better way?

share|improve this question
1  
You probably want to have wrapper return the result of replacementFunction to ensure its compatible with closure classes and prototypical ones. If this is only for prototypical class constructors you may want to choose a better name –  megawac Feb 12 '14 at 17:56
    
There's also the option of not doing anything special and just releasing a new version. There's nothing to keep users of the library from just continuing to use the current version if they don't want to upgrade. And if they do want to upgrade, give them a proper changelog and maybe an upgrade guide. –  Flambino Feb 12 '14 at 22:27

1 Answer 1

up vote 5 down vote accepted

From looking at your fiddle, the first thing I would do is make sure that your function is not anonymous:

// new function implementation
var Chart = function Chart(name, data) { //See what I did there?
    this.name = name;
    this.data = data;

    console.log("Chart > constructor");
}

Then, to keep it KISS, I would

// obsolete helper function
var ObsoleteWithReplacement = function( f ) {
    var wrapper = function() {
       console.warn("This function is now called " + f.name );
       f.apply(this, arguments);
    }
    wrapper.prototype = f.prototype;
    return wrapper;
}

Because the function is no longer anonymous, you can now use f.name, you no longer have to provide it. The drawback is that the new old function name is not logged, I think any JS developer could figure that one out.

Then you can do this:

// old deprecated function
var chart = ObsoleteWithReplacement(Chart);

The name ObsoleteWithReplacement is unfortunate, I would use simply Obsolete. Also I think function Obsolete(){..} makes more sense than var Obsolete = function Obsolete(){..}, var Obsolete = function (){..} should be avoided since it creates an anonymous function.

I forked this to http://jsfiddle.net/konijn_gmail_com/8a5Ax/

share|improve this answer
    
Thanks! Valid points on the anonymous fn's and being able to access names programmatically. However clumsy passing in the old and new function name args is, it allows for the fully-qualified-name of the fn to be put in the message. Handy if we want to include namespace. Of course whether the dev needs a more meaningful message or not is debatable. If they don't, I agree your method is cleaner but will require named functions across the project. (Not a bad thing for debugging call stack!). As for fn naming I'm really not going to get into that, we all have preferences. great help :) –  Stephen James Feb 12 '14 at 17:03

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.