4
\$\begingroup\$

The goal is to show message for users that are less 25 years old. The personal ID consists of 081010333333, where 08year, 10month, 10date. I made code and it's very messy. So the question is: how to make it "right"? any advices and solutions are welcome. One on codepen and the code itself:

$('input').on('change keyup', function(i, el){
    var d = new Date(),
            getVal = $(this).val(),         
            getYear = $(this).val().slice(0,2),
            getMonth = $(this).val().slice(2,4),
            getDay = $(this).val().slice(4,6),
            getCurrentYearFull = d.getFullYear(),
            getCurrentMonth = ("0" + (d.getMonth() + 1)).slice(-2),
            getCurrentDate = ("0" + (d.getDate())).slice(-2),
            getCurrentYearShort = d.getFullYear().toString().slice(2,4);

        if(getYear > getCurrentYearShort){
                getYear = "19" + getYear
            } else {
                getYear = "20" + getYear
            }
            var yearDiff = getCurrentYearFull - getYear;
            if (yearDiff >=10 && $('input').val().length >= 4) {
                    if(getCurrentMonth >= getMonth){
                         if (getCurrentDate >= getDay){
                             $(".check").text('good')
                         }
                        else {
                            $(".check").text('bad')
                        }
                    } else {
                        $(".check").text('bad')
                    }
            } else {
                $(".check").text('bad')

            }
        $('.test').text(yearDiff)       
})
<script src="//cdnjs.cloudflare.com/ajax/libs/jquery/2.2.2/jquery.min.js"></script>   

<input type="text" value="120330"/>
<div class="check"></div>
<div class="test"></div>

\$\endgroup\$
0
2
\$\begingroup\$

In this answer I'll just reveiw the way you calculate the age difference. So first, I would like to mention, that you should really work with four-digit numbers for displaing years - if that's possible for you.

In the suggestion below you'll see, that I don't compare the years, months and days because comparing two timestamps is better! So you just have to feed the is25YearsOld function with a yyyymmdd-date and it will tell if you are older/younger than 25.

function is25YearsOld(input) {
    // the date of birth of a 25 year old
    var dob25 = new Date();
    // setFullYear to the current year (eg. 2016) and subtract 25
    dob25.setFullYear(dob25.getFullYear() - 25);
    // set the time to midnight
    dob25.setHours(0);
    dob25.setMinutes(0);
    dob25.setSeconds(0);
    // get the unix-timestamp
    dob25 = dob25.getTime();
    // the user's date of birth
    var dob = new Date();
    // set the year, month and day accordingly to the input
    dob.setFullYear(parseInt(input.substr(0, 4)));
    dob.setMonth(parseInt(input.substr(4, 2)) - 1); // (-1) because month is zero-based
    dob.setDate(parseInt(input.substr(6, 2)));
    // set the time to midnight
    dob.setHours(0);
    dob.setMinutes(0);
    dob.setSeconds(0);
    // get the unix-timestamp
    dob = dob.getTime();
    // if the timestamp of requiredDateOfBirth is smaller than the one of dateOfBirth, the user is too young
    return dob25 >= dob;
}

// Oct 10, 2008
document.getElementById("test1").innerHTML = is25YearsOld("20081010") ? "You are old enough!" : "You are too young!";
// Oct 10, 1988
document.getElementById("test2").innerHTML = is25YearsOld("19881010") ? "You are old enough!" : "You are too young!";
<p>2008-10-10: <span id="test1"></span></p>
<p>1988-10-10: <span id="test2"></span></p>

\$\endgroup\$
1
  • \$\begingroup\$ +1 This seems cleaner than my solution as you use the Date object to do time calculations \$\endgroup\$ – jacwah Apr 1 '16 at 8:35
1
\$\begingroup\$

First, fix your formatting. Indentations in your code is not consistently one size. Making your code look good is important for improving it's readability.

Furthermore, names beginning with get are usually given to functions returning a value, most often some sort of property of an object. You prefix all your variables with get, which throws me off when reading through the code. I think this prefix can safely be removed to improve readability.

Finally, your algorithm doesn't seem correct. As I understand it, you want to show the message "bad" if a user is under 25 years old and "good" otherwise. Try these two IDs for yourself: 8902013333, 8904013333. The first person is 27 years old and the program says "good". But the second person, who is 26 years old is rejected as "bad". The problem is your use of >=, because the case were a year/month is the same and if it's greater is different.

Unfortunately we can't use moment.js to help us calculating the person's age as it counts a year as 365 days (docs). Here's an example of how you can fix your algorithm yourself:

var yearDiff = getCurrentYearFull - getYear;
var isOfAge = false;
var ageLimit = 25;
if (yearDiff == ageLimit) {
    var monthDiff = getCurrentMonth - getMonth;
    if(monthDiff == 0){
         if (getCurrentDate >= getDay){
             isOfAge = true;
         }
    } else if (monthDiff > 0) {
             isOfAge = true;
    }
} else if (yearDiff > ageLimit) {
    isOfAge = true;
}
$('.check').text(isOfAge ? 'good' : 'bad');

This also avoids all the repeated lines setting .check.

\$\endgroup\$
1
  • \$\begingroup\$ Ok, I'll look through your changes, and will get my feedback in a day. Thank you! \$\endgroup\$ – NeedHate Mar 31 '16 at 12:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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