11

What's in a name? Class names should be nouns, and although Schedule could be a noun, it looks like you chose the verb name. I think maybe 'Task' will be a better name for it? Your main method is called checkDateValid(), what does it mean? All dates passed to the method are valid... valid for what? Give the method a name which gives the context of the ...


10

private final Your fields dateStart, dateStop and the others should be marked as private final. Other classes don't need direct access to them (and normally shouldn't have direct access even if they needed it), and as they never change once the class has been initialized it's good practice to mark them as final. I'm not a big fan of the lack of line breaks ...


10

I would strongly reccomend that you use the java.time package instead of the java.util classes, if that is at all possible. One thing you could improve involves the improperly-named DAY_OF_WEEK and REPETITION enums. An enum is a type of class, and should be named using the convention for naming classes, not that for static final constants. Call them ...


8

So a couple of minor issues first: void main() Argh, bad. Someone who can write this should know better! There's a problem with your ScheduleAt function. You're passing in an rvalue reference to time, and trying to bind this to a non-const lvalue reference. Visual Studio has some (evil) extensions that let you get away with this, however, this isn't ...


7

Sometimes, comments aren't the clearest way to make a script readable: IF ( (--is it a monday? SELECT DATEPART(dw,@now) ) = 2 Consider: declare @MONDAY int; set @MONDAY = 2; if (datepart(weekday,@now) = @MONDAY) Does one need a comment to figure out what's going on? I hope not! This part however: DECLARE @now DATETIME = ...


7

No way to stop the cycle. The standard with TPL is to use the CancellationToken. Even if it is not need now, which I don't know why, it would be easier in the future if it supported it. For example if it gets turned into a service can just cancel the Token Source or if stays a console app could trap Esc key and cancel token source. Numbers for Task Delay. ...


5

As far as I see nobody reviewed the tests, so here are a few notes about that: For this: @Test public void testDateStartBeforeDateStop(){ try{ scheduleTodayTomorrow = new Schedule(tomorrow, today); Assert.fail("Should throw IllegalArgumentException when dateStart after dateStop."); } catch (IllegalArgumentException ex){ /...


5

First, I'd mention that scheduling execution of anything in Windows is better done through the Windows Task Scheduler. Scheduling your application that way would remove the "must run everyday at 4AM" requirement from the code, although I don't see how your code is enforcing that. That said... Magic Numbers & Configuration DateTime scheduleTime ...


5

Consider this improved version of Yuushi's code: #include <functional> #include <chrono> #include <future> #include <queue> #include <thread> #include <memory> #include <sstream> #include <assert.h> struct function_timer { std::function<void()> func; std::chrono::system_clock::time_point time; ...


5

1) int as time interval is ambiguous. TimeSpan is not. So I would use the latter. public int WaitTimeout { get; set; } 2) IsDisposed is a misleading name. I would not expect new HScheduler(...).IsDisposed to return true. 3) This does not look threadsafe: WaitHandle.WaitAll(waitHandles, WaitTimeout); Array.ForEach(waitHandles, (wh) => wh.Dispose()); ...


5

Imports Use import java.util.Timer; and import java.util.TimerTask; at the top of the file. This will allow you to simplify this: new java.util.Timer().schedule(new java.util.TimerTask() { to this: new Timer().schedule(new TimerTask() { which requires less typing and less reading, and so is arguably easier to understand. Naming command is a decent ...


5

Keep a single var now = DateTime.UtcNow; Initialize the check-tasks to Task.CompletedTask to get rid of all the null checks. Check if the tasks are faulted, and use Task.IsFaulted, before rescheduling the task to avoid swallowing last second exceptions. If one of the 1-minute tasks fails early the rest of the checks won't run until the task is rescheduled. ...


4

I can see two issues with your code: It doesn't work. Because the local variables filterTimeStart and filterTimeEnd are put in the closure, and the Where() lambdas are executed only when you iterate the resulting enumerables, all of your schedules will be the same as AfterTomorrowGroupedSchedules, because the last value assigned to the locals will be used. ...


4

The issues i see off the bat: 1) In my experience this Scheduler : Queue<Job> is almost always a bad idea. If you are not directly overriding a Queue class methods - there is no reason to derive Scheduler from it. Futhermore, adding/removing schedules will cause exceptions, if it happens at the same time, as you enumerate your queue in timer callback. ...


4

I don't see any issues with your switch itself. There will only ever be a finite number of arguments to be handled and this seems like a reasonable way to go about it. What I don't like is how you've repeated the check on CmdArgs.Length all over the code. (I do like your comments though. They've really clarified unclear things.) I would recommend extracting ...


4

Based on the naming guidelines input parameters should be named using camelCase casing. Calling ToString() on a String doesn't really add any value. A String is a String is a String. You have a lot of duplicated code here which can be reduced by extracting the cases of single argument calls to a separate method like Public Shared Sub ParseTasks(...


4

Variables that can be local All member variables in NotifyAlarm are only used in one method. As such, they can all be converted to local variables. It's recommended to define variables in the smallest scope necessary. In case of member variables, you need to read the entire class to understand all their uses. The same is true for MainActivity, except the ...


4

This is an interesting approach. I would have used a queue by default since it seems to express the semantics a bit clearer (the queuing is slightly more obvious). Also ContinueWith creates a Task wrapping the original task which I'm not sure if that has any form of performance downsides (it probably shouldn't). I hacked a quick benchmark together with the ...


4

Here are some things that may help you improve your code. Be careful with signed and unsigned In the doit routine and various others, the code compares an int i to a size_t num_tasks, but size_t is unsigned and int is signed. Instead, declare both variables as size_t types. Also, see the next suggestion. Be careful with namespace assumptions The C++ ...


3

The code does not match the description of the requirements. It calculates the time based on 12:45 PM, instead of 4:00 AM. One problem with your posting is that it's not immediately clear which Timer you are using. (For edification of anyone else reading this, the most likely are System.Timers.Timer or System.Threading.Timer; the other possible Timers are ...


3

It looks/sounds like you're trying to build a sort of thread pool. If that's the case, take a look at this StackExchange link for some help on what it is (a thread design pattern). For your purposes you essentially have a worker queue (a single threaded thread pool), where the only difference is (as you are already doing) you use a mutex object to block the '...


3

First, threading bugs: The way you're accessing _jobs from multiple threads is not thread-safe! You need to ensure that whenever you're reading or writing to the list, it's under a lock (probably under a different lock than _sync, since you don't want to wait for all current jobs to execute before a call to AddJob() returns. private bool _paused; Since ...


3

Just a couple of notes. public void PauseScheduler() { if (!_paused) { _paused = !_paused; } } public void ResumeScheduler() { if (_paused) { _paused = !_paused; } } Can be public void PauseScheduler() { _paused = true; } public void ResumeScheduler() { _paused = false; } The test here is unnecessary if (...


3

I can't find anything wrong with this. Just ... nitpicks. You name your variables in for each loops t. for (T t : supplier.tasks()) { taskQueue.add(t); } I think if you gave them a better name like task, it would improve readability. Your test code seems to be on emergency rationing for variable name characters. private ...


3

Your question would do better with a more detailed description. The way I understand your situation is: you accumulate a bunch of 'notifications' that need to be mailed in the database on a fixed schedule you find all the notifications that are now 'due', and you 'pull' them in using List<T> tasks = this.getTasksForExecution(now); you then process ...


3

Your script creates a temporary file unnecessarily, uses os.system which is not recommended, and other messy things. Consider this alternative: #!/usr/bin/env python import subprocess import re re_job_number = re.compile(r'\d+') for atq_line in subprocess.Popen(['atq'], stdout=subprocess.PIPE).stdout: job_number = re_job_number.search(atq_line).group(...


3

Never User.all.each in the real world. That loads your entire table into memory and instantiates ActiveRecord models for every record. You want User.find_each do |user|, which uses find_in_batches to load 1000 records at a time. Don't do simple filtering in Ruby, do it in the database. This... if user.reminder == true should be a simple scope that ...


3

Don't write custom scripts for common sysadmin tasks. Using the proper tools will get the job done better, and the solution will be more maintainable — especially by any colleagues you might have. Using monit, for example, this would just be a matter of writing a simple configuration file. set logfile /root/process_monitor.log check process myproc with ...


3

public static IObservable<DateTime> Create(TimeSpan interval, IDateTime dateTime) { return Observable .Interval(TimeSpan.FromSeconds(1)) .Select(_ => dateTime.Now()); } You forgot to plug interval parameter. And, I would consider renaming it resolution. I wouldn't trust the values from selecting DatetTime::Now ...


3

If this is intended to be run as a command, I recommend you add a suitable shebang line. Although the question is tagged bash, there's nothing here that isn't portable POSIX shell, so I recommend #!/bin/sh Is it intentional that the paths all share a common initial prefix /volume1 and that the log and error paths share a longer common prefix? If so, ...


Only top voted, non community-wiki answers of a minimum length are eligible