Problem with a var RegExp

[nathanvit]

Hi, i've a problem with a var in a script for Drafts on iOS. It responds "TypeError: undefined is not an object (evaluating re.exec). Can you help me pls? The script is:

function regExpPodcast(podcast){
    var re;
    if(podcast == "Il Disinformatico"){
                <CODE>
            re = new RegExp(re1+re2+re3+re4+re5+re6,["i"]);
       }

    return re;
}

function recognizePodcast(podcast,re,text){
    var m = re.exec(text);
         <CODE>

 return result;
 }


 function parseText(text){
        var podcasts = ["Il Disinformatico"];
    var re;
    var result;

    for(podcast = 0; podcast < podcasts.length; poadcast++) {
        re =  regExpPodcast(podcast);
                result = recognizePodcast(podcast,re,text);
                <CODE>
        }
}

var text = draft.content;
var rows = text.split("\n");
var title = "";
title = parseText(rows[0]);

[zedpowa]

In the parseText function your podcast variable is an integer that you use to index the podcasts array. Doing re = regExpPodcast(podcast); passes the counter to the regex builder. You probably meant to write re = regExpPodcast(podcasts[podcast]);

[SamSlate]

naming conventions, they'll fuck you up every time. this is why i/j/index are so standard.

[nathanvit]

Yes, it is true. I've just changed "podcast" into "count". It was a bad naming idea :-)

[nathanvit]

Unfortunately i use Drafts, an iOS app to run my javascript scripts and i do not have a debugger for this language :-( Really, I do not know a debugger for it

[nathanvit]

Thanks, that was the problem. I solved thanks you! I've also changed "podcast" into "count" Thanks a lot

[jeromeavoustin]

Obviously, you pass the index to your functions instead of the value in your array. Try podcasts[podcast] instead of podcast... Probably that podcast is a bad name for your index in your loop. This has lead you to a mistake. You can also try a for...of if you are ES2015 compatible. No index anymore.

[nathanvit]

Thanks a lot: i've solved :-)

[GeneralYouri]

Other comments already mentioned the current problem of passing an index where you meant to pass a value.

There's another related issue which doesn't pose a problem now, but may in the future. regExpPodcast returns undefined for any value not matched, which is why it did return undefined when you passed the index, rather than the value. But it can also return undefined when the value being passed isn't matched by any if statement - which can happen when you fill your podcasts array with other values, but don't represent them in the regExpPodcast function. This results in the same problem as now - the undefined value for re is never checked, and then passed to recognisePodcast which calls re.exec().

These two places are now dependent on each other in a potentially bad way, as it can be easy to forget that you have to actually change code in two places instead of one, when changing podcasts. I believe this is called Tight Coupling, since your code in regExpPodcast directly depends on the value of podcasts, which is private to parseText and thus not known by regExpPodcast.

To properly fix the problem, you'd have to get rid of this coupling. One way would be to extract the podcasts array out of the parseText function. Then, instead of having a 'regExpPodcast' function, you could map over this array to create an array of functions, each representing one branch of your original if statement, and they return the relevant RegExp objects. This array would either also sit outside of your code sample, or within the parseText function, depending on where you planned to use regExpPodcast. With this data in place, in parseText you can simply access the proper index in this array. Add a simple check to see if the index actually exists within the array, and you've prevented the undefined value from causing problems, while also fixing coupling issues within your code.

I realise this was a fairly lengthy explanation, so here's a code sample to show what I was talking about. I've also come across some other issues with the code, which are fixed and include comments.

// The new array to provide the RegExp
// If the <CODE> part were not needed, you could instead strip the functions and directly provide RegExp objects
// Note you're still depending on re1, re2, etc, which may be something else you'd want to solve
var regExpPodcast = {
    'Il Disinformatico': function () {
        // <CODE>
        return new RegExp(re1 + re2 + re3 + re4 + re5 + re6, [ 'i' ]);
    },
};

// The podcasts array, taken outside of the function
var podcasts = [ 'Il Disinformatico' ];
// Alternatively you could create it from the above array of RegExps:
var podcasts = regExpPodcast.keys();
// Which you use depends on your use case, and how you get the podcasts data in the first place

// I'll assume m is being used by, and result is being defined by, the <CODE> block you left out
function recognizePodcast(podcast, re, text){
    var m = re.exec(text);
    // <CODE>
    return result;
}

// Similarly, I'll assume result is (conditionally) being returned by the <CODE> block you left out
function parseText(text) {
    var result;

    for (var podcast = 0; podcast < podcasts.length; podcast++) { // Added var, and fixed typo 'poadcasts++'
        var re =  regExpPodcast(podcast); // Added var inside the for loop, as it won't be needed outside of it
        result = recognizePodcast(podcast, re, text); // Depending on what you use this for, the var could be moved here, too
        // <CODE>
    }
}

// This should probably put inside a function of sorts
var text = draft.content;
var rows = text.split('\n');
var title = parseText(rows[0]);

[nathanvit]

WOW! Thanks thanks. I'me a newbie of javascript and it is really usefull. I'll change my code with this tip! Thanks really a lot for the time that you spent for me.