add getDate() method for #80

[cbhat5]

…m the Personnummer object

Type of change

• New feature
• Bug fix
• Security patch
• Documentation update

Description

Add the getDate method for #80

Related issue

Closes #80

Motivation

All good.

Checklist

• I have read the CONTRIBUTING document.
• I have read and accepted the Code of conduct
• Tests passes.
• Style lints passes.
• Documentation of new public methods exists.

[Johannestegner]

Thank you for the pull request!

The code looks good, I'd just love a check on the actual output in the tests and I'll happily merge it :)

[cbhat5]

@Johannestegner The existing code with below test case sets the fullYear to be 2009 instead of 1909 for separated_format/short format. I create LocalDateTimeobject based on fullYear attribute. The base code needs to be rewritten to consider the space.

{
        "integer": 0,
        "long_format": "19090527 1474",
        "short_format": "090527 1474",
        "separated_format": "090527 1474",
        "separated_long": "19090527 1474",
        "valid": false,
        "type": "ssn",
        "isMale": true,
        "isFemale": false
    }

I am currently using

				Integer expected = Integer.valueOf(ssn.longFormat.substring(0,4));
				int actual = entry.getDate().getYear();
				
				assertEquals(expected, actual, "expected = " + expected + "\n" + "Actual = " + actual);

and can switch to the following if the existing logic is sure to be correct:

				Integer expected = Integer.valueOf(getFullYear());
				int actual = entry.getDate().getYear();
				
				assertEquals(expected, actual, "expected = " + expected + "\n" + "Actual = " + actual);

[Johannestegner]

When it comes to the short format, we always have to guess which century the number is for, so that one is expected.
When it comes to the separated format, if there is a -, the number will be seen as within the 100 year range, and a + over 100 years.

So the full year should be correct to use.

Easiest way to handle this in the tests would be to just check on the decade value instead of the full year in the assert :)

[Johannestegner]

In the C# package, I format the value from the Date object as a yyMMdd value and just check them against each other, which I think is a sufficient test.

[cbhat5]

When it comes to the short format, we always have to guess which century the number is for, so that one is expected. When it comes to the separated format, if there is a -, the number will be seen as within the 100 year range, and a + over 100 years.

So the full year should be correct to use.

Easiest way to handle this in the tests would be to just check on the decade value instead of the full year in the assert :)

Will not the short format also have a separator either of - or + instead of the in the short format?

[Johannestegner]

There are basically four formats we support as input:

long: 198510033999
long with separator: 19851003-3999
short: 8510033999
short with separator: 851003-3999

The short we can only guess the century on, the other three we can get it either from the first 2 chars (19) or through the separator (-/+).

There should be no space, and if the data downloaded from the meta repository by the tests contains that there must be an error.

I'll double check that :)

[Johannestegner]

Aaah I double checked the meta (and then again looked at the code you shown) and that number is invalid (due to the space), so it should fail in the parse even :)

[cbhat5]

Yes, the last one does have space which should be an error then. Anyways, I have used getFullYear for assertin.

[Johannestegner]

I was thinking checking all the four parts here, YY MM and DD.
Just against a simple format would work fine:

// something like...
assertEquals(ssn.longFormat.substring(2, 6),  date.getYear() + date.getMonth() + date.getDay());

Although, I'd guess that the date object have some type of format function with a string value to pass (yyMMdd would do in that case I would imagine).

With that I'll happily merge the PR :)

[cbhat5]

Ok, I'll add the dates and months. I totally forgot with this century issue popping up.

[cbhat5]

Aaah I double checked the meta (and then again looked at the code you shown) and that number is invalid (due to the space), so it should fail in the parse even :)

yeah, looks like an issue with regex, I added a space as an additional option, and it fails many other tests, I am not sure of the other possible patterns. Does the updated PR look fine to you? It passes the tests with getFullYear.

[Johannestegner]

Yes, looks good to me! :)

Thank you for the pull request!