0

I'm trying to create something like an info screen that just scrolls through images (and maybe one day pdfs) that are uploaded with a small managment app I wrote.

After some reading I think I understand why I should NOT have Thread.sleep in my for loop. I read some posts on stackoverflow and other pages that teached me not to do it.

Apparently I'm supposed to use ExecutorService for something like this, but I can't wrap my head around it.

So after preparing my GUI and everything else I finally call my showImages and stay in this while loop. After each loop I reload the records from the file systems, so any changes are represented on the screen.

I created a metafile containing the preferred delay for each image and just sleep for the amount of time.

private void showImages() {
        //noinspection InfiniteLoopStatement
        while (true) {
            loadRecords();
            for (ImageRecord record : records) {
                System.out.println("Showing: " + record.getFilename());
                imageLabel.setIcon(record.getIconForInfoScreen(screenWidth, screenHeight));
                int delay = record.getDurationAsInt();
                System.out.println("Waiting for " + delay + " milliseconds");
                try {
                    Thread.sleep(delay);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
        }
    }

So if I understand correctly I could use something like

ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
exec.scheduleAtFixedRate(showImage(),0, delay, TimeUnit.MILLISECONDS);

But this would trigger the showImage() every "delay" millisecond and I still need to increment some counter, to get to the next image in my records.

Could someone push me in the right direction? I'm kinda lost at the moment.

Ovski
  • 575
  • 3
  • 14
  • When you get an image, update the counter, so it will be ready for the next image. – cliff2310 Dec 03 '21 at 19:33
  • 1
    *Apparently I'm supposed to use ExecutorService* - if you want animation you should be using a `Swing Timer`. Check out: https://stackoverflow.com/questions/33907207/how-to-make-jscrollpane-in-borderlayout-containing-jpanel-smoothly-autoscroll/33907282#33907282 for a simple example. – camickr Dec 03 '21 at 22:23
  • So, a larger issue is, Swing is not thread safe. So you shouldn't be updating the UI from outside the context of the Event Dispatching Thread. You could make use of a `SwingWorker` which could driver the primary workflow, this could be added to an executor service, if desired, or you could just have it run within its own context, but you'd still end up with a `Thread.sleep`. Can you elaborate the reasons why you think you shouldn't use `Thread.sleep`? – MadProgrammer Dec 03 '21 at 22:46
  • I think I've figured it out. The link from camickr was showing an example that I've seen may times, but never really could implement. Now, as I got rid of my while and for loop I finaly made it work. The missing piece was the fact that I should not use any loop but use the timer instead. – Ovski Dec 06 '21 at 06:35
  • @MadProgrammer one reason is the fact, that a thread.sleep pauses my gui and at this time can block keyboard listeners. To be honest, I was pretty happy with how my application was working, other then the keyboardlisteners sometimes taking mulitple presses to react. But the IDE and the internet told me to fix this issue. So now the keypresses are smoother and the IDE is happy :) – Ovski Dec 06 '21 at 06:40
  • @SimonRoth So, that's not a reason to avoid `Thread.sleep`, it's just a reason not to do it in the context of the Event Dispatching Thread, which makes Camickr's comment a reasonable suggestion ;) – MadProgrammer Dec 06 '21 at 07:26
  • @MadProgrammer Agreed, it's wrong to state that one should avoid Thread.sleep at all cost. And yes I recoded my method to implement the information vom camickr's link. Because I couldn't acceppt a comment as an answer I just answered my question myself. – Ovski Dec 06 '21 at 12:48

1 Answers1

0

All the comments on my question can be considered good ideas an and one of them also lead me to my final result. But it in the end, the matter was, that I did not understand how to implement a timer into my loop.

Or in other words, the loops had to be gone and instead use a counter and set the delay for the follow up record inside the timer loop.

    private void loopImages() {
        loadRecords();
        recordSize = records.size();
        int delay = records.get(1).getDurationAsInt();
        timer = new Timer(delay, e -> showImages());
        timer.start();
    }

    private void showImages() {
        if (recordIndex == recordSize) {
            loadRecords();
            recordSize = records.size();
            recordIndex = 0;
        }

        ImageRecord record = records.get(recordIndex);

        imageLabel.setIcon(record.getIconForInfoScreen(screenWidth, screenHeight));

        recordIndex++;
        if (recordIndex < recordSize) {
            record = records.get(recordIndex);
            int delay = record.getDurationAsInt();
            timer.setDelay(delay);
        }
    }
Ovski
  • 575
  • 3
  • 14