patternjavaspringMinor
Bot-like Spring Java service for checking advertisements
Viewed 0 times
springadvertisementscheckinglikejavabotservicefor
Problem
I am in the middle of developing a Spring Boot application, which has a service acting like a bot.
Immediately after the start of the app, it invokes the method
If a new Advertisement comes up, then it calls
I am wondering if the solution (I mean the whole service class) is proper and if it can be beter?
```
package com.gadomski.service;
import com.gadomski.model.Advertisement;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
import java.util.Random;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
/**
* Created by Krzysiek on 21.08.2016.
*/
@Service
public class CheckerService {
private final Logger log = LoggerFactory.getLogger(this.getClass());
private final int CHECKER_THREADS = 100;
@Autowired
AdvertisementService advertisementService;
private ScheduledExecutorService scheduledExecutorService;
CheckerService() {
this.scheduledExecutorService = Executors.newScheduledThreadPool(CHECKER_THREADS);
}
public void scheduleCheck(int delaySeconds, Advertisement advertisement) {
if (advertisement.isActive()) {
ScheduledFuture check = this.scheduledExecutorService.schedule(
() -> {
scheduleCheck(getDelayInSeconds(), advertisement);
advertisementService.updateAdvertisement(advertisement);
log.info("Scheaduled next check for " + advertisement.getUrl()
advertisementService.update(Advertisement ad) is a method which parses some HTML. It is important that it cannot execute periodically. For now it is pseudo random, but in the future there will be some logic returning the time depending on information read in previous parsing.Immediately after the start of the app, it invokes the method
scheduleAllChecks().If a new Advertisement comes up, then it calls
scheduleCheck(0, ad);I am wondering if the solution (I mean the whole service class) is proper and if it can be beter?
```
package com.gadomski.service;
import com.gadomski.model.Advertisement;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
import java.util.Random;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
/**
* Created by Krzysiek on 21.08.2016.
*/
@Service
public class CheckerService {
private final Logger log = LoggerFactory.getLogger(this.getClass());
private final int CHECKER_THREADS = 100;
@Autowired
AdvertisementService advertisementService;
private ScheduledExecutorService scheduledExecutorService;
CheckerService() {
this.scheduledExecutorService = Executors.newScheduledThreadPool(CHECKER_THREADS);
}
public void scheduleCheck(int delaySeconds, Advertisement advertisement) {
if (advertisement.isActive()) {
ScheduledFuture check = this.scheduledExecutorService.schedule(
() -> {
scheduleCheck(getDelayInSeconds(), advertisement);
advertisementService.updateAdvertisement(advertisement);
log.info("Scheaduled next check for " + advertisement.getUrl()
Solution
It appears that you are using
Instances of
You should probably rewrite this to use
When applicable, use of
Random across multiple threads. I guess you plan to replace it in the future, but still, according to the documentation:Instances of
java.util.Random are threadsafe. However, the concurrent use of the same java.util.Random instance across threads may encounter contention and consequent poor performance. Consider instead using ThreadLocalRandom in multithreaded designs.You should probably rewrite this to use
java.util.concurrent.ThreadLocalRandom. As the docs say:When applicable, use of
ThreadLocalRandom rather than shared Random objects in concurrent programs will typically encounter much less overhead and contention.Context
StackExchange Code Review Q#147240, answer score: 2
Revisions (0)
No revisions yet.