Repository: go-co-op/gocron
Pull request #26 (2020-04-11 19:39:09)

add periodDuration to Job & and refactor schedule next run & others
- we were defining periodDuration every time we scheduled the next run, moved that into the Job struct so we set it once when the `Do` func is called
- renamed a bunch of functions, moving away from `Get/Set` naming


Patch:
diff --git a/go.mod b/go.mod
index 0605225..5baf982 100644
--- a/go.mod
+++ b/go.mod
@@ -9,4 +9,4 @@ require (
 	github.com/stretchr/testify v1.4.0
 	golang.org/x/net v0.0.0-20200226121028-0de0cce0169b // indirect
 	golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e // indirect
-)
\ No newline at end of file
+)
diff --git a/job.go b/job.go
index 3a262d0..8d1f4e9 100644
--- a/job.go
+++ b/job.go
@@ -8,9 +8,10 @@ import (
 // Job struct stores the information necessary to run a Job
 type Job struct {
 	interval          uint64                   // pause interval * unit between runs
+	unit              timeUnit                 // time units, ,e.g. 'minutes', 'hours'...
+	periodDuration    time.Duration            // interval * unit
 	startsImmediately bool                     // if the Job should run upon scheduler start
 	jobFunc           string                   // the Job jobFunc to run, func[jobFunc]
-	unit              timeUnit                 // time units, ,e.g. 'minutes', 'hours'...
 	atTime            time.Duration            // optional time at which this Job runs
 	err               error                    // error related to Job
 	lastRun           time.Time                // datetime of last run
@@ -72,40 +73,39 @@ func (j *Job) Tags() []string {
 	return j.tags
 }
 
-func (j *Job) periodDuration() (time.Duration, error) {
+func (j *Job) setPeriodDuration() error {
 	interval := time.Duration(j.interval)
-	var periodDuration time.Duration
 
 	switch j.unit {
 	case seconds:
-		periodDuration = interval * time.Second
+		j.periodDuration = interval * time.Second
 	case minutes:
-		periodDuration = interval * time.Minute
+		j.periodDuration = interval * time.Minute
 	case hours:
-		periodDuration = interval * time.Hour
+		j.periodDuration = interval * time.Hour
 	case days:
-		periodDuration = interval * time.Hour * 24
+		j.periodDuration = interval * time.Hour * 24
 	case weeks:
-		periodDuration = interval * time.Hour * 24 * 7
+		j.periodDuration = interval * time.Hour * 24 * 7
 	default:
-		return 0, ErrPeriodNotSpecified
+		return ErrPeriodNotSpecified
 	}
-	return periodDuration, nil
+	return nil
 }
 
-// NextScheduledTime returns the time of the Job's next scheduled run
-func (j *Job) NextScheduledTime() time.Time {
+// ScheduledTime returns the time of the Job's next scheduled run
+func (j *Job) ScheduledTime() time.Time {
 	return j.nextRun
 }
 
-// GetScheduledTime returns the specific time of day the Job will run at
-func (j *Job) GetScheduledTime() string {
+// ScheduledAtTime returns the specific time of day the Job will run at
+func (j *Job) ScheduledAtTime() string {
 	return fmt.Sprintf("%d:%d", j.atTime/time.Hour, (j.atTime%time.Hour)/time.Minute)
 }
 
-// GetWeekday returns which day of the week the Job will run on and
+// Weekday returns which day of the week the Job will run on and
 // will return an error if the Job is not scheduled weekly
-func (j *Job) GetWeekday() (time.Weekday, error) {
+func (j *Job) Weekday() (time.Weekday, error) {
 	if j.unit == weeks {
 		return j.startDay, nil
 	}
diff --git a/scheduler.go b/scheduler.go
index 876340c..e35952f 100644
--- a/scheduler.go
+++ b/scheduler.go
@@ -68,8 +68,8 @@ func (s *Scheduler) Less(i, j int) bool {
 	return s.jobs[j].nextRun.Unix() >= s.jobs[i].nextRun.Unix()
 }
 
-// SetLocation changes the default time location
-func (s *Scheduler) SetLocation(newLocation *time.Location) {
+// ChangeLocation changes the default time location
+func (s *Scheduler) ChangeLocation(newLocation *time.Location) {
 	s.loc = newLocation
 }
 
@@ -77,17 +77,12 @@ func (s *Scheduler) SetLocation(newLocation *time.Location) {
 func (s *Scheduler) scheduleNextRun(j *Job) error {
 	now := s.time.Now(s.loc)
 
-	periodDuration, err := j.periodDuration()
-	if err != nil {
-		return err
-	}
-
 	switch j.unit {
 	case seconds, minutes, hours:
-		j.nextRun = j.lastRun.Add(periodDuration)
+		j.nextRun = j.lastRun.Add(j.periodDuration)
 	case days:
 		j.nextRun = s.roundToMidnight(j.lastRun)
-		j.nextRun = j.nextRun.Add(j.atTime).Add(periodDuration)
+		j.nextRun = j.nextRun.Add(j.atTime).Add(j.periodDuration)
 	case weeks:
 		j.nextRun = s.roundToMidnight(j.lastRun)
 		dayDiff := int(j.startDay)
@@ -100,7 +95,7 @@ func (s *Scheduler) scheduleNextRun(j *Job) error {
 
 	// advance to next possible Schedule
 	for j.nextRun.Before(now) || j.nextRun.Before(j.lastRun) {
-		j.nextRun = j.nextRun.Add(periodDuration)
+		j.nextRun = j.nextRun.Add(j.periodDuration)
 	}
 
 	return nil
@@ -112,7 +107,7 @@ func (s *Scheduler) roundToMidnight(t time.Time) time.Time {
 }
 
 // Get the current runnable Jobs, which shouldRun is True
-func (s *Scheduler) getRunnableJobs() []*Job {
+func (s *Scheduler) runnableJobs() []*Job {
 	var runnableJobs []*Job
 	sort.Sort(s)
 	for _, job := range s.jobs {
@@ -143,7 +138,7 @@ func (s *Scheduler) Every(interval uint64) *Scheduler {
 
 // RunPending runs all the Jobs that are scheduled to run.
 func (s *Scheduler) RunPending() {
-	runnableJobs := s.getRunnableJobs()
+	runnableJobs := s.runnableJobs()
 	for _, job := range runnableJobs {
 		s.runAndReschedule(job) // we should handle this error somehow
 	}
@@ -199,8 +194,8 @@ func (s *Scheduler) Remove(j interface{}) {
 	})
 }
 
-// RemoveByRef removes specific Job j by reference
-func (s *Scheduler) RemoveByRef(j *Job) {
+// RemoveByReference removes specific Job j by reference
+func (s *Scheduler) RemoveByReference(j *Job) {
 	s.removeByCondition(func(someJob *Job) bool {
 		return someJob == j
 	})
@@ -259,17 +254,20 @@ func (s *Scheduler) Do(jobFun interface{}, params ...interface{}) (*Job, error)
 	j.fparams[fname] = params
 	j.jobFunc = fname
 
-	if !j.startsImmediately {
-		periodDuration, err := j.periodDuration()
+	if j.periodDuration == 0 {
+		err := j.setPeriodDuration()
 		if err != nil {
 			return nil, err
 		}
+	}
+
+	if !j.startsImmediately {
 
 		if j.lastRun == s.time.Unix(0, 0) {
 			j.lastRun = s.time.Now(s.loc)
 
 			if j.atTime != 0 {
-				j.lastRun = j.lastRun.Add(-periodDuration)
+				j.lastRun = j.lastRun.Add(-j.periodDuration)
 			}
 		}
 


Test patch:
diff --git a/example_test.go b/example_test.go
index 0afc134..7ba6b22 100644
--- a/example_test.go
+++ b/example_test.go
@@ -23,9 +23,9 @@ func ExampleScheduler_At() {
 	s.Every(1).Monday().At("10:30:01").Do(task)
 }
 
-func ExampleJob_GetScheduledTime() {
+func ExampleJob_ScheduledTime() {
 	s := gocron.NewScheduler(time.UTC)
 	job, _ := s.Every(1).Day().At("10:30").Do(task)
-	fmt.Println(job.GetScheduledTime())
+	fmt.Println(job.ScheduledAtTime())
 	// Output: 10:30
 }
diff --git a/job_test.go b/job_test.go
index 613479d..5d4e3bc 100644
--- a/job_test.go
+++ b/job_test.go
@@ -22,7 +22,7 @@ func TestTags(t *testing.T) {
 
 func TestGetScheduledTime(t *testing.T) {
 	j, _ := NewScheduler(time.UTC).Every(1).Minute().At("10:30").Do(task)
-	assert.Equal(t, "10:30", j.GetScheduledTime())
+	assert.Equal(t, "10:30", j.ScheduledAtTime())
 }
 
 func TestGetWeekday(t *testing.T) {
@@ -42,7 +42,7 @@ func TestGetWeekday(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			weekday, err := tc.job.GetWeekday()
+			weekday, err := tc.job.Weekday()
 			if tc.expectedError != nil {
 				assert.Error(t, tc.expectedError, err)
 			} else {
@@ -53,3 +53,29 @@ func TestGetWeekday(t *testing.T) {
 		})
 	}
 }
+
+func TestSetPeriodDuration(t *testing.T) {
+
+	testCases := []struct {
+		desc             string
+		job              *Job
+		expectedDuration time.Duration
+		expectedError    error
+	}{
+		{"seconds", &Job{interval: 1, unit: seconds}, time.Duration(1) * time.Second, nil},
+		{"minutes", &Job{interval: 1, unit: minutes}, time.Duration(1) * time.Minute, nil},
+		{"hours", &Job{interval: 1, unit: hours}, time.Duration(1) * time.Hour, nil},
+		{"days", &Job{interval: 1, unit: days}, time.Duration(1) * time.Hour * 24, nil},
+		{"weeks", &Job{interval: 1, unit: weeks}, time.Duration(1) * time.Hour * 24 * 7, nil},
+		{"none", &Job{interval: 1}, 0, ErrPeriodNotSpecified},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			err := tc.job.setPeriodDuration()
+			assert.Equal(t, tc.expectedError, err)
+			assert.Equal(t, tc.expectedDuration, tc.job.periodDuration)
+		})
+	}
+
+}
diff --git a/scheduler_test.go b/scheduler_test.go
index b7b797a..c020bd8 100644
--- a/scheduler_test.go
+++ b/scheduler_test.go
@@ -64,7 +64,7 @@ func TestStartImmediately(t *testing.T) {
 	now := time.Now().UTC()
 
 	job, _ := sched.Every(1).Hour().StartImmediately().Do(task)
-	next := job.NextScheduledTime()
+	next := job.ScheduledTime()
 
 	nextRounded := time.Date(next.Year(), next.Month(), next.Day(), next.Hour(), next.Minute(), next.Second(), 0, time.UTC)
 	expected := time.Date(now.Year(), now.Month(), now.Day(), now.Hour(), now.Minute(), now.Second(), 0, time.UTC)
@@ -87,7 +87,7 @@ func TestAt(t *testing.T) {
 
 	// Expected start time
 	expectedStartTime := time.Date(now.Year(), now.Month(), now.Day(), now.Hour(), now.Minute(), now.Add(time.Second*2).Second(), 0, time.UTC)
-	nextRun := dayJob.NextScheduledTime()
+	nextRun := dayJob.ScheduledTime()
 	assert.Equal(t, expectedStartTime, nextRun)
 
 	stop := s.StartAsync()
@@ -97,7 +97,7 @@ func TestAt(t *testing.T) {
 
 	// Expected next start time 1 day after
 	expectedNextRun := expectedStartTime.AddDate(0, 0, 1)
-	nextRun = dayJob.NextScheduledTime()
+	nextRun = dayJob.ScheduledTime()
 	assert.Equal(t, expectedNextRun, nextRun)
 }
 
@@ -117,13 +117,13 @@ func TestAtFuture(t *testing.T) {
 
 	// Check first run
 	expectedStartTime := time.Date(now.Year(), now.Month(), now.Day(), now.Hour(), now.Add(time.Minute).Minute(), 0, 0, time.UTC)
-	nextRun := dayJob.NextScheduledTime()
+	nextRun := dayJob.ScheduledTime()
 	assert.Equal(t, expectedStartTime, nextRun)
 
 	s.RunPending()
 
 	// Check next run's scheduled time
-	nextRun = dayJob.NextScheduledTime()
+	nextRun = dayJob.ScheduledTime()
 	assert.Equal(t, expectedStartTime, nextRun)
 	assert.Equal(t, false, shouldBeFalse, "Day job was not expected to run as it was in the future")
 }
@@ -196,9 +196,9 @@ func TestWeekdays(t *testing.T) {
 
 	job1, _ := scheduler.Every(1).Monday().At("23:59").Do(task)
 	job2, _ := scheduler.Every(1).Wednesday().At("23:59").Do(task)
-	t.Logf("job1 scheduled for %s", job1.NextScheduledTime())
-	t.Logf("job2 scheduled for %s", job2.NextScheduledTime())
-	assert.NotEqual(t, job1.NextScheduledTime(), job2.NextScheduledTime(), "Two jobs scheduled at the same time on two different weekdays should never run at the same time")
+	t.Logf("job1 scheduled for %s", job1.ScheduledTime())
+	t.Logf("job2 scheduled for %s", job2.ScheduledTime())
+	assert.NotEqual(t, job1.ScheduledTime(), job2.ScheduledTime(), "Two jobs scheduled at the same time on two different weekdays should never run at the same time")
 }
 
 // This ensures that if you schedule a job for today's weekday, but the time is already passed, it will be scheduled for
@@ -210,13 +210,13 @@ func TestWeekdaysTodayAfter(t *testing.T) {
 
 	runTime := fmt.Sprintf("%02d:%02d", timeToSchedule.Hour(), timeToSchedule.Minute())
 	job, _ := getWeekday(scheduler.Every(1), time.UTC).At(runTime).Do(task)
-	t.Logf("job is scheduled for %s", job.NextScheduledTime())
-	if job.NextScheduledTime().Weekday() != timeToSchedule.Weekday() {
+	t.Logf("job is scheduled for %s", job.ScheduledTime())
+	if job.ScheduledTime().Weekday() != timeToSchedule.Weekday() {
 		t.Errorf("Job scheduled for current weekday for earlier time, should still be scheduled for current weekday (but next week)")
 	}
 	nextWeek := time.Date(now.Year(), now.Month(), now.Day()+7, now.Hour(), now.Minute()-1, 0, 0, time.UTC)
-	if !job.NextScheduledTime().Equal(nextWeek) {
-		t.Errorf("Job should be scheduled for the correct time next week.\nGot %+v, expected %+v", job.NextScheduledTime(), nextWeek)
+	if !job.ScheduledTime().Equal(nextWeek) {
+		t.Errorf("Job should be scheduled for the correct time next week.\nGot %+v, expected %+v", job.ScheduledTime(), nextWeek)
 	}
 }
 
@@ -230,8 +230,8 @@ func TestWeekdaysTodayBefore(t *testing.T) {
 
 	runTime := fmt.Sprintf("%02d:%02d", timeToSchedule.Hour(), timeToSchedule.Minute())
 	job, _ := getWeekday(scheduler.Every(1), time.UTC).At(runTime).Do(task)
-	t.Logf("job is scheduled for %s", job.NextScheduledTime())
-	if !job.NextScheduledTime().Equal(timeToSchedule) {
+	t.Logf("job is scheduled for %s", job.ScheduledTime())
+	if !job.ScheduledTime().Equal(timeToSchedule) {
 		t.Error("Job should be run today, at the set time.")
 	}
 }
@@ -377,7 +377,7 @@ func TestRemoveByRef(t *testing.T) {
 
 	assert.Equal(t, 2, scheduler.Len(), "Incorrect number of jobs")
 
-	scheduler.RemoveByRef(job1)
+	scheduler.RemoveByReference(job1)
 	assert.ElementsMatch(t, []*Job{job2}, scheduler.Jobs())
 }
 
@@ -434,7 +434,7 @@ func TestSetLocation(t *testing.T) {
 
 	assert.Equal(t, time.FixedZone("UTC-8", -8*60*60), s.loc)
 
-	s.SetLocation(time.UTC)
+	s.ChangeLocation(time.UTC)
 
 	assert.Equal(t, time.UTC, s.loc)
 }