3
\$\begingroup\$

Can you please check if I've written the code correctly?

The task was:

  • Calculate the user's month of birth as a a number, where January = 0 through to December = 11.
  • Take the string entered
  • Get the substring, being the first three characters
  • Convert to uppercase
  • Find the starting location of the three-letter abbreviation in the month abbreviations string, and divide this by 3 (this is not the only way to find the month number, but it allows us to practice searching in a string)

var year = prompt('Enter year of birth as a 4 digit integer');      

var month = prompt('Enter the name of the month of birth');

// Chop everything after the first 3 characters and make it uppercase
month = month.substring(0,3).toUpperCase();

// Store your array in months, differently named than the month input
var months = ["JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", "OCT",         
"NOV", "DEC"];

// We then use array.indexOf() to locate it in the array
var pos = months.indexOf(month);
if (pos >= 0) {
// valid month, number is pos
}

var date = prompt('Enter day of birth as an integer');
\$\endgroup\$
13
  • 2
    \$\begingroup\$ @Zirak I've run it, and it works properly. But As I've said, I'm new to programming, and I want to make sure I've covered everything. \$\endgroup\$
    – George Li
    Jul 26, 2012 at 11:33
  • \$\begingroup\$ @ThiefMaster thanks for that, but I have to write what I know and I'm not familiar with the code you've wrote :( \$\endgroup\$
    – George Li
    Jul 26, 2012 at 11:44
  • \$\begingroup\$ "... and make it lowercase" vs. .toUpperCase(). Which one is it? \$\endgroup\$ Jul 26, 2012 at 17:51
  • \$\begingroup\$ @JosephSilber - Sorry, my mistake, it's to uppercase. \$\endgroup\$
    – George Li
    Jul 27, 2012 at 4:46
  • \$\begingroup\$ @GeorgeLi The last bullet seems to imply that the month names are to be stored in a string, not an array - is that required? \$\endgroup\$
    – Inkbug
    Jul 27, 2012 at 6:39

3 Answers 3

1
\$\begingroup\$

Here is the whole code; explanations after it.

(function () {

  var year, month, months, pos, date;

  year = prompt( "Enter year of birth as a 4 digit integer" );
  month = prompt( "Enter the name of the month of birth" );

  months = "JANFEBMARAPRMAYJUNJULAUGSEPOCTNOVDEC";

  pos = months.indexOf( month.substring( 0, 3 ).toUpperCase() );

  if ( pos === -1 ) {
    alert( "Invalid month name: " + month );
  } else {
    alert( "Month number: " + ( 1 + pos / 3 ) );
  }

  date = prompt( "Enter day of birth as an integer" );
})();

(function () {
...
})();

The above code is called an Immediately-Invoked Function Expression (IIFE), and is used to not pollute the global namespace with all of our variables. Variables declared with a var are only accessible inside the IIFE. Other styles of IIFEs are also used (such as with a semicolon before it), however this is what I prefer.

  var year, month, months, pos, date;

It is considered good practice to put all variable declarations at the top. This makes for better minification (when the code is used in production), and to prevent errors do to misunderstanding of variable hoisting.

  year = prompt( "Enter year of birth as a 4 digit integer" );
  month = prompt( "Enter the name of the month of birth" );

For a better user experience, it might have been better to use a form, but I don't think that would be worth the effort. Note in addition that I standardized the quotes (" everywhere), and that I used spaces inside parentheses (different coding styles differ; this is my preferred one).

  months = "JANFEBMARAPRMAYJUNJULAUGSEPOCTNOVDEC";

This is the names of the month (first three letters) in one string.

  pos = months.indexOf( month.substring( 0, 3 ).toUpperCase() );

This gets the (zero based) location of the searched month. Note that your way is more robust (for example, try an input of Unjvember), but that this seems to be what was asked for.

  if ( pos === -1 ) {
    alert( "Invalid month name: " + month );
  } else {
    alert( "Month number: " + ( 1 + pos / 3 ) );
  }

The requirements didn't ask explicitly for an alert telling the number, but I put it in anyway.

  date = prompt( "Enter day of birth as an integer" );

This (and the first prompt for a year) was in your original code, so I left it in (formatted).

\$\endgroup\$
0
5
\$\begingroup\$

Everything looks good.

That said, array.indexOf wasn't supported in Internet Explorer until version 9.

If you need that support, the Mozilla Developer Network has a handy function you can include in your code to provide support for those browsers:

if (!Array.prototype.indexOf) {
    Array.prototype.indexOf = function (searchElement /*, fromIndex */ ) {
        "use strict";
        if (this == null) {
            throw new TypeError();
        }
        var t = Object(this);
        var len = t.length >>> 0;
        if (len === 0) {
            return -1;
        }
        var n = 0;
        if (arguments.length > 0) {
            n = Number(arguments[1]);
            if (n != n) { // shortcut for verifying if it's NaN
                n = 0;
            } else if (n != 0 && n != Infinity && n != -Infinity) {
                n = (n > 0 || -1) * Math.floor(Math.abs(n));
            }
        }
        if (n >= len) {
            return -1;
        }
        var k = n >= 0 ? n : Math.max(len - Math.abs(n), 0);
        for (; k < len; k++) {
            if (k in t && t[k] === searchElement) {
                return k;
            }
        }
        return -1;
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ Thanks for your reply Danny. Let me get this straight, var index = array.indexOf(2); should be included in my script? So it would look like this: var index = array.indexOf(2); var pos = months.indexOf(month); if (pos >= 0) { } \$\endgroup\$
    – George Li
    Jul 27, 2012 at 0:24
  • \$\begingroup\$ @GeorgeLi - No, you would actually keep your code the same, but instead add the provided block of code above your current code. (See updated answer). That said, if this code is for a class and your teacher didn't say anything about this, your original code is probably correct enough. \$\endgroup\$ Jul 27, 2012 at 3:29
  • \$\begingroup\$ Thanks for that. It's just a tasks I'm doing from an online tutorial. I probably won't need to include it. Anyhow, so my code fits all the requirements of the task? I just want to double check. \$\endgroup\$
    – George Li
    Jul 27, 2012 at 3:37
0
\$\begingroup\$

I think you're commenting way too much. Comments that just repeat what the code is doing without any more insight just serve to confuse the reader more; for example, saying that toUpperCase() makes the string uppercase when the function name already makes that clear isn't very helpful.

If you're writing these comments to help you remember what functions do, kind of like notes, then I guess that's fine. But when you start writing code other people will read, remember to leave that stuff out.


Your code right now doesn't handle a user's mistakes very well. If I were to write some junk input by accident (like 'banana'), then I'd have no way to go back and fix things. Here I used infinite while loops that only exit when the input is legit:

var year, month, day;
var months = ["JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", "OCT", "NOV", "DEC"];

while (true) {
    year = parseInt(prompt('Enter year of birth as a 4 digit integer'));

    /* Check that YEAR is a 4-digit number. */
    if (year && (year + "").length === 4) {
         break; /* This will exit out of this infinite loop. */
    }
}

while (true) {
    month = prompt('Enter the name of the month of birth');
    month = month.substring(0,3).toUpperCase();

    /* Check that MONTH is an actual month (within MONTHS.) */
    if (months.indexOf(month) >= 0) {
         break;
    }
}

while (true) {
    day = prompt('Enter day of birth as an integer');

    /* Check that DAY is between 1 and 31. */
    if (day >= 1 && day <= 31) {
        break;
    }
}

alert("Your birthday: "+ month +" "+ day +" "+ year);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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