It all started with a git commit I found:
commit 340d908fe537d3163e220c84945bbfc5b49fe8dc
Author: A.
Date: Wed Apr 14 17:28:50 2021 +0200
Deleted redundant INFO messages. Replaced date strings with dt->ymd() calls. Replaced some if nots with unless. Simplified sub _unpack. Set timezone in DateTime objects. Results are returned as tail calls now. Maybe more minor changes.
A. is a fresh developer in my department and as many future devs she did not learn so far - neither in university nor in books - how to organize her work. How to avoid commits ending with Maybe more minor changes. Honestly I know a lot senior devs who still use a version control system like any other backup. So what I wanted to show her, was:
- How to organize my programming in small steps
- How slowly developing
- How to make errors without desaster
- How to use your version control
- How to write a helpful commit-message
- How will I later find something I did
So I started a programming-session on my own, and gave her later the repository and we discussed each step. (Honestly: She found immediately an error in my code, that I did not left there intentionally).
The development was in perl, but I hope other devs find the steps valueable too, so I write it down here. (My first longer article in english)
Step one: all started with a README
- Script with parameters
-d Date
-> has to be in the past
-> must be no older then 7 days
Of course that was the first commit:
commit
Author: Volker Kroll
Date: Wed May 5 16:49:17 2021 +0200README
- we will have the requirements here
Now the fun started, the initial script
use strict;
use Getopt::Long;
my $date ; ## the startingdate
GetOptions(
"date=s" => \$date,
);
print "submitted Date is: $date\n";
The perl people know, that script does not much more then storing a submitted string as parameter --date or -d in the scalar variable $date.
And of course it uses strict as every good perl script does.
I committed this change:
Author: Volker Kroll
Date: Wed May 5 16:52:01 2021 +0200initial
- Parameter d for the date
TODO:
- Validation
As promised a validation
## starts as before
sub is_date {
my $date = shift;
return 1 if $date =~ /^\d\d\d\d-\d\d-\d\d$/;
return 0;
}
if(is_date($date)) {
print "submitted Date is: $date\n";
}
else {
print "submitted value $date is no date\n";
}
I checked if the submitted date matches the regex ^\d\d\d\d-\d\d-\d\d$ - you are aware, that we perl-devs solve our problems with regexes right?
But enough for the next commit
Author: Volker Kroll
Date: Wed May 5 16:54:49 2021 +0200first check for date
- date is valid if regex \d\d\d\d-\d\d-\d\d is valid
TODO:
- better check
- exclude non valid month and day
More checks
As announced the next step was to better check for the validity of the date - and stay tuned these steps will be changed later.
sub is_date {
my $date = shift;
my $days = ["", 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31];
my($y, $m, $d) = $date =~ /^(\d\d\d\d)-(\d\d)-(\d\d)$/;
if ($m > 0 && $m < 13) {
print "$m has max $days->[$m] days\n";
return 1 if $d <= $days->[$m];
}
return 0;
}
And the next commit even though theses checks are far from perfect
Author: Volker Kroll
Date: Wed May 5 17:00:23 2021 +0200more checking of date via regex
- added length per month value
- check if day is possible in the month
TODO:
- that is a bad choice for february
Let's correct the february
As we saw, my checks did not really work in february - only in leap-years, so I had to handle that with a check wether it is a leap year or not.
sub is_date {
my $date = shift;
my $days = ["", 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31];
my($y, $m, $d) = $date =~ /^(\d\d\d\d)-(\d\d)-(\d\d)$/;
if (is_leap_year($y)) {
$days->[2] = 29;
}
if ($m > 0 && $m < 13) {
print "$m has max $days->[$m] days\n";
return 1 if $d <= $days->[$m];
}
return 0;
}
sub is_leap_year {
my $y = shift;
if ($y % 4 == 0) {
return 1 if $y % 400 == 0;
return 0 if $y % 100 == 0;
return 1;
}
}
Now february is handled correctly in every year. So, up the next commit
Author: Volker Kroll
Date: Wed May 5 17:07:06 2021 +0200check works even for february
- new function is_leap_year TODO
- Refactoring - Removing of all own written calendar functions
Refactoring - Use of Date Modules
I had to remove my not so elegant and error-prone date-functions. To get you back on track, here is the whole script after removing that and adding a suitable date module
use strict;
use Getopt::Long;
use DateTime;
my $date ; ## the startingdate
GetOptions(
"date=s" => \$date,
);
sub is_date {
my $date = shift;
my $days = ["", 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31];
my($y, $m, $d) = $date =~ /^(\d\d\d\d)-(\d\d)-(\d\d)$/;
my $dt;
eval {
$dt = DateTime->new(year => $y, month => $m, day => $d);
};
if ($@) {
print STDERR "Error while generating DateTime: $@\n";
return 0;
}
else {
return $dt;
}
}
if(is_date($date)) {
print "submitted Date is: $date\n";
}
else {
print "submitted value $date is no date\n";
}
I am using now DateTime for every date operation. If no DateTime object can be created we assume, that the submitted value is not correct and return 0, else we return a DateTime Object.
Author: Volker Kroll
Date: Wed May 5 17:24:41 2021 +0200removed own calendar functions and added DateTime
- validation via Exception of DateTime->new()
TODO:
- Refactoring: rename function is_date (returns a dt if true)
Maybe you saw, that I forgot to remove the arry with the days per month... I saw it now, while I write this article
Clean it up 1/2
use strict;
use Getopt::Long;
use DateTime;
my $date; ## the starting date argument
my $dt; ## DateTime Object of starting date
GetOptions(
"date=s" => \$date,
);
sub get_dt {
my $date = shift;
my $days = ["", 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31];
my($y, $m, $d) = $date =~ /^(\d\d\d\d)-(\d\d)-(\d\d)$/;
my $dt;
eval {
$dt = DateTime->new(year => $y, month => $m, day => $d);
};
if ($@) {
print STDERR "Error while generating DateTime: $@\n";
return 0;
}
else {
return $dt;
}
}
$dt = get_dt($date);
if($dt) {
print "submitted Date is: $date\n";
}
else {
print "submitted value $date is no date\n";
}
Everything I did is mentioned in the commit-message (and there is still that array of days...
Author: Volker Kroll
Date: Wed May 5 17:28:04 2021 +0200moved function is_date to get_dt
- check is now if return of get_dt is a true value a DateTime Object is a true value 0 is not => so the check still works => so we have already a working DateTime Object to work with later
Clean it up 2/2
Now I moved around the code a bit to make it more readable
use strict;
use Getopt::Long;
use DateTime;
my $date; ## the starting date argument
my $dt; ## DateTime Object of starting date
GetOptions(
"date=s" => \$date,
);
$dt = get_dt($date);
if($dt) {
print "submitted Date is: $date\n";
}
else {
print "submitted value $date is no date\n";
}
The sub get_dt remained the same (and there is still that array)
Author: Volker Kroll
Date: Wed May 5 17:31:00 2021 +0200moved main code up in the file
Now for more functionality
As we remember the day has to be in the past. So we need to check that. DateTime Objects can easily be compared and DateTime has a function called today(). So my first step was:
$dt = get_dt($date);
my $today = DateTime->today();
if(!$dt) {
print "submitted value $date is no date\n";
}
if($dt < $today) {
print "submitted date is in the past\n";
}
That was easy and it worked out of the box.
Author: Volker Kroll
Date: Wed May 5 17:33:56 2021 +0200check if submitted date is in the past
- added new DateTime Object $today
- compare $today and DateTime Object of submitted date
TODO
- next check: not older then 7 days (see README)
But first let's remove this array....
I think you know how a not existing line looks nevertheless that was worth a commit
Author: Volker Kroll
Date: Wed May 5 17:37:45 2021 +0200removed old arrayref for days in the month
Check if it was in the last week
In our requirements was that the date has to be in the last seven days. (Whatever that means in detail)
Time for another Date Module DateTime::Duration to calculate that difference.
if(is_last_week($dt)) {
print "submitted date was in the last week\n";
}
sub is_last_week {
my $dt = shift;
return unless ref $dt eq "DateTime";
my $dur = DateTime::Duration->new(days => 7);
$dt->add_duration($dur);
return 1 if $today < $dt;
}
I think that code is easily to understand even for not perl-devs. I add 7 days to the submitted date and compare it with today. (Here is the error that my colleague found immediatly - but more about the bug later).
Author: Volker Kroll
Date: Wed May 5 17:45:57 2021 +0200check for is_last_week
- added DateTime::Duration
- checked if submitted date was in the last 7 days
TODO:
- Refactoring: Add the check for in the past in is_last_week
Cleanup
As mentioned in the TODO we removed the "is in the past" check from the main and added it to the is_last_week
sub is_last_week {
my $dt = shift;
return unless ref $dt eq "DateTime";
return if($dt > $today) ;
my $dur = DateTime::Duration->new(days => 7);
$dt->add_duration($dur);
return 1 if $today < $dt;
}
Normally all this would end with the this commit:
Author: Volker Kroll kroll@strato.de
Date: Wed May 5 17:48:31 2021 +0200is_last_week now a valid check for date concerning to specification
The bug
As I wrote before, my colleague spotted a bug in my code. I got this question via chat:
A: The line $dt->add_duration($dur); made me wonder. Is the dt object not changed then? Is this call by value or call by reference?
And she was right. So the next commit was from her, modifying my code and correcting it.
diff --git a/skript.pl b/skript.pl
index 2f5594b..426c4af 100644
--- a/skript.pl
+++ b/skript.pl
@@ -30,7 +30,8 @@ sub is_last_week {
return if($dt > $today) ;
my $dur = DateTime::Duration->new(days => 7);
- $dt->add_duration($dur);
+ #$dt->add_duration($dur);
+ $today->subtract_duration($dur);
return 1 if $today < $dt;
}
She changed the direction of the math and modified the no longer used $today
Author: A.
Date: Fri May 7 12:38:15 2021 +0200is_last_week subtracts from today instead of adding to dt.
Now the script was correct concerning the requirements.
Conclusions
Of course this is a little bit artificial - and most devs will need to confess that they usually commit much bigger chunks of code. Nevertheless, I think it is valuable to develop in these small chunks.
Next steps were making it a bit more modular and added automated tests for further development. But for today this shall be enough.
Top comments (0)