Daily coding problem: Job scheduler invoking a function after specific time

Question

This is a problem from Daily Coding Problem. I have implemented it in Java. For a function f I have used Runnable interface.

Problem:

This problem was asked by Apple.

Implement a job scheduler which takes in a function f and an integer n, and calls f after n milliseconds.

Solution:

import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;

class Ideone {

    interface JobScheduler {
        void schedule(final Runnable f, int n);
    }

    static class JobSchedulerImpl implements JobScheduler {
        private final AtomicInteger idValue = new AtomicInteger(0);
        private final Map<Integer, Thread> jobs = new ConcurrentHashMap<>();

        private int getNextId() {
            return idValue.incrementAndGet();
        }

        @Override
        public void schedule(final Runnable f, final int n) throws IllegalArgumentException {
            if (n < 0) {
                throw new IllegalArgumentException("n cannot be nagative");
            }
            int id = getNextId();
            Thread job = new Thread() {
                @Override
                public void run() {
                    try {
                        Thread.sleep(n);
                        f.run();
                        jobs.remove(id);
                    } catch (InterruptedException e) {
                        jobs.remove(id);
                    }
                }
            };
            jobs.put(id, job);
            job.start();
        }
    }

    public static void main(String[] args) {
        JobScheduler jobScheduler = new JobSchedulerImpl();
        jobScheduler.schedule(new Runnable() {
            @Override
            public void run() {
                System.out.println("Job1 launched at: " + System.currentTimeMillis());
            }
        }, 10000);
        jobScheduler.schedule(new Runnable() {
            @Override
            public void run() {
                System.out.println("Job2 launched at: " + System.currentTimeMillis());
            }
        }, 1000);
    }
}
```

Answer 1

Basics

First of all, you've got the basics right, being naming conventions, indentation, and a rather sane OOP structure. But you ask for review, so here I go.

Reinventing the wheel

You probably know the Timer class, doing exactly what you implemented.

So I guess, your program is meant as an exercise.

Use proper names

Your class is named Ideone, and that's terrible. You didn't create a kind of IDE. [If this class name is somehow mandated by your environment, move away from that as fast as possible.]

User a proper package name. Placing code into the unnamed (top-level) package is a bad idea, as it invites name collisions with others.

void schedule(final Runnable f, int n);

Single-letter variable names tell nothing about their meaning. A better naming would be

void schedule(final Runnable job, int millisecondsFromNow);

You use the variable name job for the thread waiting for and later executing the function, which I'd call jobExecutionThread instead.

Don't use wildcard imports

import java.util.*;

This can create problems in the long run. Imagine the Java stewards to include a new class into the java.util package in some future version, one that collides with some other class name you reference. The effect can be that your code will no longer compile with this future Java version.

You can easily avoid that by having your IDE manage the imports for you (and disallow any wildcard imports in the IDE settings, if necessary).

Interface JobScheduler

You create an inner interface JobScheduler inside you class Ideone, and that way, it's useless.

If you want to allow for more implementations of JobScheduler (although I don't see a point in it), move it to top-level.

For its current use, just inside the top-level class Ideone with exactly one implementation, I'd simply use the implementing class, and get rid of the interface.

The jobs management

You invested quite some effort into naming the jobs with a synthetic integer id and maintaining a map of the jobs. What for?

You only use the id in

jobs.remove(id);

For removing an object from a collection, you don't need this integer, you can just maintain a Collection and remove the thread itself

private final Collection<Thread> jobs = ...;

jobs.remove(this);

But why maintain such a collection at all? Currently, there's nothing you do with it. It would make sense if you had an info call returning a list of pending jobs or something like that. But as long as this doesn't exist, get rid of the jobs management stuff.

And, regarding integer ids generally: to me, that's remains from ancient times when the only way to machine-identify an object was thought to be by numbering. First of all, objects have an identity of their own (in Java supported by equals() and hashCode() methods, if necessary), and if something additional is necessary, using numbers often creates more problems than necessary. See for example the internet. Nobody would try to use integers as web resource identities, the URL strings do a much better job.

Javadoc

Get into the habit of documenting important aspects of your code, especially interfaces, classes, and public methods. Your team colleagues (and your future self after one year) will really appreciate that. Follow the Javadoc style for these comments, as it's well supported in IDEs and other tools.