Comp Sci How Can I Ensure Messages in Java ArrayList Are Sorted by Time Descending Order?

[Mark44]

The code in post #29 doesn't print anything, so it's possible there's a bug in your routine that prints the contents of the message list.

Your for loop needs some adjustment.
1) Assuming there are n messages in the message list, i ranges from 0 to n - 1. When i == n - 1, you are check message[n-1] and message[n], which is past the end of your list.
2) If you find the right place to add the message, insert the message and then bail out of your loop. I mentioned this back in post #17.

You could put the if statement above inside a for loop, and break out of the loop (with break;) when the if condition is met.

 

[blah45]

What do you do if the if condition is not met? All possible scenarios are checked

It's probably very oblivious to you what to do but I've been at this one method for quite some time that I'm a bit distrought with it anyway..

 public void addMsg(Message MsgToAdd)
    {    
        int i = 0 ;
        boolean found = false ;  
        Message MsgInList ;
        if(MessageList.size() == 0)
        {
            MessageList.add(MsgToAdd) ;
        }

        else if(MessageList.size() == 1)
        {
            MsgInList = MessageList.get(i) ;
            if(MsgToAdd.getTimePc() > MsgInList.getTimePc())
            {
                MessageList.add(0,MsgToAdd) ;
            }
            else
            {
                MessageList.add(1,MsgToAdd) ;
            }
        }
        else
        {  

            for(i = 0 ; i == MessageList.size() - 1 ; i++)
            {
                MsgInList = MessageList.get(i) ;
                if((MsgToAdd.getTimePc() < MsgInList.getTimePc()) && MsgToAdd.getTimePc() > MessageList.get(i + 1).getTimePc())
                {                   
                    MessageList.add(i+1,MsgToAdd) ;                   
                } 
            }
        }
    }

 

[blah45]

It's still diplaying only two messages..out of 5

 

[Mark44]

You have an error here:
for(i = 0 ; i ==[/color] MessageList.size() - 1 ; i++)

That should be
for(i = 0 ; i < MessageList.size() - 1 ; i++)

 

[Mark44]

It's still diplaying only two messages..out of 5

What's your routine to print the contents of a message list?

 

[blah45]

I have a driver class that has a main method

public class messageDriver
{
    public static void main(String args[])
    {
        MessageList Inbox = new MessageList() ;

        Message msg1 = new Message("123456","Hi") ;

        Message msg2 = new Message("123456","Hey was up") ;

        Message msg3 = new Message("123456","Nothing much") ;

        Message msg4 = new Message("123456","k bye") ;

        Message msg5 = new Message("123456","bye bye") ;
        
        Inbox.addMsg(msg4) ;
        Inbox.addMsg(msg1) ;
        Inbox.addMsg(msg5) ;
        Inbox.addMsg(msg2) ;
        Inbox.addMsg(msg3) ;
        Inbox.display() ;
        
    }
}

 

[Mark44]

What's the display() method look like?

 

[blah45]

public void display()
    {
        if(MessageList.size() == 0)
        {
            System.out.println("There is no messages in the list") ;    
        }
        else 
        {
            int top = 0 ;
            int j = MessageList.size() ;
            for(int i = 0 ; i < MessageList.size() ; i++ )
            {

                Message msg = MessageList.get(top) ;
                System.out.println("Message " + j + " is ---> " +  msg.getMessage()) ;
                j-- ;
                top++ ;

            }
        }
    }

 

[Mark44]

Is there some reason you can't just do this?

public void display()
{
    int count = MessageList.size();
    if(count == 0)
    {
        System.out.println("There are no messages in the list") ;    
    }
    else 
    {
        Message msg;
        for(int i = 0 ; i < count; i++ )
        {
            msg = MessageList.get(i) ;
            System.out.println("Message " + i + " is ---> " +  msg) ;
        }
    }
}

 

[blah45]

I did that and it still only print's out 2 messages but object references to those messages

Message 0 is ---> Message@c70b0d
Message 1 is ---> Message@1a6a1a7

 

[Mark44]

Slight change -

public void display()
{
    int count = MessageList.size();
    if(count == 0)
    {
        System.out.println("There are no messages in the list") ;    
    }
    else 
    {
        Message msg;
        for(int i = 0 ; i < count; i++ )
        {
            msg = MessageList.get(i) ;
            System.out.println("Message " + i + " is ---> " +  msg.message[/color]) ;
        }
    }
}

What do you have that you can debug with? You need to be able to look at the memory where you message list is being stored and see whether there are 5 messages in it or just 2. You also need to be able to watch as the 5 messages get added to the message list.

 

[blah45]

I'm using a program called bluej and it has a built in debugger, when it get's to the the 3rd add message method in the driver the if doesn't execute and then the last two do execute, then when it goes into the display the count was equal to 4 not five but this time it displayed 3 messages with the text in the correct order...so yeah. does that help you in anyway

 

[Mark44]

So of your 5 messages only 4 are going into your message list?

 

[blah45]

4 go in but only 3 get displayed

 

[blah45]

It's weird 3 are displayed when I run while debugging it, but when the driver runs it only 2 are displayed

 

[blah45]

Once again the full code is...

   public void addMsg(Message MsgToAdd)
    {    
        int i = 0 ;
        boolean found = false ;  
        Message MsgInList ;
        if(MessageList.size() == 0)
        {
            MessageList.add(MsgToAdd) ;
        }

        else if(MessageList.size() == 1)
        {
            MsgInList = MessageList.get(i) ;
            if(MsgToAdd.getTimePc() > MsgInList.getTimePc())
            {
                MessageList.add(0,MsgToAdd) ;
            }
            else
            {
                MessageList.add(1,MsgToAdd) ;
            }
        }
        else
        {  

            for(i = 0 ; i < MessageList.size() - 1 ; i++)
            {
                MsgInList = MessageList.get(i) ;
                if((MsgToAdd.getTimePc() < MsgInList.getTimePc()) && MsgToAdd.getTimePc() > MessageList.get(i + 1).getTimePc())
                {                   
                    MessageList.add(i+1,MsgToAdd) ;    
                    break ;
                }                
            }
        }
    }

 

[Mark44]

It's possible that the timestamps are showing equal times, which is something Grep mentioned yesterday. You could add this line in your display() method.

System.out.println("Message time is " msg.timeReceived) ;

I think this would work, but I can't remember if your timeReceived property is public or private. If it's private, the above won't work, but I'm not sure whether to trust your getTimePc() method.

EDIT:
I went back and looked at the code you posted in post #16, and saw that timeReceived is private. getTimePc() seems to be fine, as it uses nanoTime() to get the time. To display the message timestamp, use

System.out.println("Message time is " msg.getTimePc()) ;

 

[blah45]

The time stamps are the same..

[ messageDriver.main({ }) ]
This message is from 123456 at 1:36:59am
Hi
This message is from 123456 at 1:36:59am
Hey was up
This message is from 123456 at 1:36:59am
Nothing much
This message is from 123456 at 1:36:59am
k bye
This message is from 123456 at 1:36:59am
bye bye

Message time is 1301017019306
Message 0 is ---> k bye
Message time is 1301017019306
Message 1 is ---> Hi

 

[Mark44]

That's what I thought. In your driver, try this:

public class messageDriver
{
    public static void main(String args[])
    {
        MessageList Inbox = new MessageList() ;

        Message msg1 = new Message("123456","Hi") ;
        Thread.sleep(250);

        Message msg2 = new Message("123456","Hey was up") ;
        Thread.sleep(250);

        Message msg3 = new Message("123456","Nothing much") ;
        Thread.sleep(250);

        Message msg4 = new Message("123456","k bye") ;
        Thread.sleep(250);

        Message msg5 = new Message("123456","bye bye") ;
        Thread.sleep(250);

        Inbox.addMsg(msg4) ;
        Inbox.addMsg(msg1) ;
        Inbox.addMsg(msg5) ;
        Inbox.addMsg(msg2) ;
        Inbox.addMsg(msg3) ;
        Inbox.display() ;
      
    }
}

sleep(n) is a static method on the Thread class that puts a thread to sleep (pauses it) for n milliseconds. Being that it's a static method, you don't call it on an object - you call it from the name of the class. In each of the calls to sleep above, I'm telling the thread to pause for 1/4 second.

 

[blah45]

Whe I put that in I get an erroe saying " unreported exception java.lang.InterruptedException ; must be caught or declared to be thrown

 

[blah45]

I just used a for loop and added delay that way and it worked =D

 

[blah45]

I'll do some other testing and let you know Thanks for all the help, thanks so much, I really do appreciate it.

 

[Grep]

Whe I put that in I get an erroe saying " unreported exception java.lang.InterruptedException ; must be caught or declared to be thrown

Best you know how to handle that, and why it's happening. Trust me, it'll come up again.

Look at the Thread.sleep method's signature in the Java API docs:

public static void sleep(long millis)
                  throws InterruptedException

So it declares that it may throw that exception and that it must be handled (and tells you the two ways to handle it).

As with any exception, it may be caught. So you can put a try/catch block around the calls to Thread.sleep. Just wrap the part with sleep calls with something like this:

try {
    // Bunch of code
    Thread.sleep(250);
    // etc ...
} catch (InterruptedException e) {
    // Handle the exception
    e.printStackTrace();
}

That makes the compiler happy. Having a throws clause in a method signature forces the programmer using it to handle the exception.

But you can also pass the buck and make any method that calls yours (the one with the sleeps in it) handle it. It will needs to be handled somewhere down the line, but sometimes it's appropriate to let a calling method handle it. In which case you just add a throws clause to your own method. For example:

public void tester() throws InterruptedException
{
    Thread.sleep(250);
    // ... etc ...
}

Now any method calling tester() will need to handle catching the exception (or passing the buck with another throws).

And thanks to Mark for pointing out it's static. Been a few years since I've used threads in Java and somehow had the idea in my head that it had to be used in a Thread/Runnable class. Which is not the case, since it's static and we're just running in the main thread.

Well, hope that clears it up. It's quite common that an API method will have a throws clause, so you'll need to know it soon enough.

 

[Mark44]

I'll do some other testing and let you know Thanks for all the help, thanks so much, I really do appreciate it.

You're welcome. I know that both Grep and I enjoy being able to help out.

 

[blah45]

I'm back again, it works but just wondering do I have to do the Thread.sleep(n) thing or is this(i.e what I have at the moment) okay...

public class messageDriver
{
    public static void main(String args[])
    {
        MessageList Inbox = new MessageList() ;
        int i = 0;        
        Message msg1 = new Message("123456","Hi") ;
        
        for(i = 0 ; i < 10000000 ; i++);
        Message msg2 = new Message("123456","Hey, how are you") ;
        
        for(i = 0 ; i < 10000000 ; i++);
        Message msg3 = new Message("123456","grand, you?") ;
        
        for(i = 0 ; i < 10000000 ; i++);
         Message msg4 = new Message("123456","I'm okay,gotta go now, talk later!") ;
       
        for(i = 0 ; i < 10000000 ; i++);
        Message msg5 = new Message("123456","ok bye") ;
        
        for(i = 0 ; i < 10000000 ; i++);
        Message msg6 = new Message("123456","bye bye") ;
        
        for(i = 0 ; i < 10000000 ; i++);

        Inbox.addMsg(msg5) ;
        Inbox.addMsg(msg1) ;
        Inbox.addMsg(msg3) ;
        Inbox.addMsg(msg6) ;
        Inbox.addMsg(msg2) ;        
        Inbox.addMsg(msg4) ;
        
        Inbox.display() ;

    }
}

 

[Grep]

I'm back again, it works but just wondering do I have to do the Thread.sleep(n) thing or is this(i.e what I have at the moment) okay...

Like I said earlier, the for loop thing works, but it's kind of cheesy.

Sleep is fine. Or also appropriate would be what I proposed earlier with the while loop. it just waits until it sees the timer has incremented and is now different than the one from the previous message. Then it exits the while loop and goes on to the next message.

If you look back a bit you can find it. But yeah, sleep is also totally appropriate. I, personally, would be a bit embarrassed to hand in a program using for loops for delays like that. I'd only use it for a quick temporary test. But it's up to you.

And thanks. Like Mark said, glad to help.

 

[blah45]

I was looking up the API for the thread but I don't exactly know how to use it in the driver, does it really matter apart from being "cheesy"?

 

[Grep]

I was looking up the API for the thread but I don't exactly know how to use it in the driver, does it really matter apart from being "cheesy"?

Well, it could be an issue, in theory. Maybe your computer in the future will be fast enough so that it does 1,000,000 iterations before the clock can update. Who knows. Probably not an issue, but it's so easy to do it better.

Here, I'll show you how I might do it more cleanly because I think you can learn a lot from seeing it. I'll use the while loop method I mentioned because it gets to the core of the issue. The clock must update between messages. And may as well use loops to arrange a sequence of messages.

    public static void main(String args[])
    {
        MessageList inbox = new MessageList() ;
        int numMessages = 6;
        int testOrder[] = {4, 0, 2, 5, 1, 3};
        Message messages[] = new Message[numMessages];

        // Create the messages, making sure system clock changes before next message
        for (int i = 0; i < numMessages; i++)
        {
            messages[i] = new Message(Integer.toString(i), "Message " + i);

            long messageTime = messages[i].getTimePc();
            while (System.nanoTime() == messageTime);
        }

        // Add messages to list in the order specified by the testOrder[] array
        for (int i = 0; i < numMessages; i++)
        {
            inbox.add(messages[testOrder[i]]);
        }

        inbox.display();
    }

I'd also add some code to test it and tell me if it's working ok or not (messages in expected order, and the right number of messages). But I suppose no need if it wasn't asked for.

Anyways, hope you found that enlightening.

 

[Grep]

I wanted to add that there's a simpler way I can think of to write the add method. I wrote nicely commented code that does it, but it wouldn't be proper to just post it, IMO.

However, I'll post the comments for the three logic branches in my if/else-if/else block. It'll still be up to you to turn it into nice code to fix what you have now. I hope it will help at least untangle the logic in your mind.

1. Always keeping in mind that the messages are ordered from newest to oldest in the list, first, I did this:
// Is list empty? Then just add.

2. If the list wasn't empty, I checked another special case:
// Is new message older than oldest message at end of list?
// Then just append to end of list (early part)

3. Otherwise, I had a simple case of iterating over *all* the elements one by one from the start with this logic:
// Otherwise we will need to find the element we are newer than
// starting from the beginning of the list with the newest times.
// If we're greater than it, then we've found our position and
// insert ourselves in front of the element. If there was a
// newer element in the list, we would have gotten to it already
// earlier in the list (where times are newer).
(EDIT: I believe the comparison on this last case should be greater than OR EQUAL TO. Keep that in mind.)

So you see that handles all the possibilities as far as I can see. I'm tired and might have made a mistake, but I present this logic to you for your consideration. Seems to work and makes good logical sense, I think.