refactor runtime check & typings of exclusive parameters timeZone & utcOffset

[gomez-git]

Does it? First condition precludes the second

https://github.com/kelektiv/node-cron/blob/a7dee32091b91dcf465e67c32a57d66151b5a774/src/job.ts#L56 https://github.com/kelektiv/node-cron/blob/a7dee32091b91dcf465e67c32a57d66151b5a774/src/job.ts#L57

https://github.com/kelektiv/node-cron/blob/a7dee32091b91dcf465e67c32a57d66151b5a774/src/job.ts#L65

[sheerlox]

hey @gomez-git :wave: thank you for your suggestion!

I liked the solution you first suggested:

if (timeZone != null && utcOffset != null) {
	throw new ExclusiveParametersError('timeZone', 'utcOffset');
} else if (timeZone != null) {
	this.cronTime = new CronTime(cronTime, timeZone, null);
} else {
	this.cronTime = new CronTime(cronTime, null, utcOffset);
}

but then I thought (again), why not do:

if (timeZone != null && utcOffset != null) {
	throw new ExclusiveParametersError('timeZone', 'utcOffset');
} else {
	this.cronTime = new CronTime(cronTime, null, utcOffset);
}

the reason we can't is that even though TypeScript knows timeZone and utcOffset are mutually exclusive in the CronJobParams interface, it seems impossible to implement a similar behavior outside of interfaces (like two variables or function arguments).

so I'm thinking of a helper function, which would take timeZone and utcOffset as parameters, throw the error if both are defined and run the if/else if/else logic you suggested to return an object {timeZone, utcOffset} were they are mutually exclusive (like in CronJobParams).

this way we centralize the timeZone != null && utcOffset != null and error throwing into a single place, and that would theoretically allow us to refactor this horror I made into a single new CronJob call :tada:

would you be interested in submitting a PR for this change? this repository is going to be part of Hacktoberbest, so if you're planning to participate we can register that issue/PR as a participation :wink:

[rharshit82]

picking this up

[Victor1890]

Hi @gomez-git and @sheerlox, I have seen that issue and this is my proposal to refactor code using your suggestion

PR: #841