[sip-comm-dev] Re: svn commit: r5309 - impl/protocol/icq


#1

Hey Lubo,

That was a great catch. I had made a similar slip in
ContactGroupJabberImpl which is why the build was failing the last
several days. Wonder why they didn't catch this one.

Thanks for noticing!
Cheers
Emil

lubomir_m@dev.java.net wrote:

···

Author: lubomir_m
Date: 2009-04-30 10:41:06+0000
New Revision: 5309

Modified:
   trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java
   trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java
   trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java

Log:
Fixes programming errors detected by FindBugs.

Modified: trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java?view=diff&rev=5309&p1=trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java&p2=trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java&r1=5308&r2=5309

--- trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java (original)
+++ trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java 2009-04-30 10:41:06+0000
@@ -12,7 +12,7 @@
import net.kano.joustsim.oscar.oscar.service.ssi.*;

/**
- * The ICQ implementation of the ContactGroup interface. Intances of this class
+ * The ICQ implementation of the ContactGroup interface. Instances of this class
  * (contrary to <tt>RootContactGroupIcqImpl</tt>) may only contain buddies
  * and cannot have sub groups. Note that instances of this class only use the
  * corresponding joust sim source group for reading their names and only
@@ -33,7 +33,7 @@
      * strings in the left column because screen names in AIM/ICQ are not case
      * sensitive.
      */
- private Map<String, Contact> buddies
+ private final Map<String, Contact> buddies
         = new Hashtable<String, Contact>();

     private boolean isResolved = false;
@@ -71,7 +71,7 @@
      * to directly compare ( == ) instances of buddies we've stored and others
      * that are returned by the framework.
      * <p>
- * @param joustSimGroup the JoustSIM Group correspoinding to the group
+ * @param joustSimGroup the JoustSIM Group corresponding to the group
      * @param groupMembers the group members that we should add to the group.
      * @param ssclCallback a callback to the server stored contact list
      * we're creating.
@@ -153,16 +153,25 @@
      */
     void removeContact(ContactIcqImpl contact)
     {
- buddies.remove(contact);
+ for (Iterator<Map.Entry<String, Contact>> buddyIter
+ = buddies.entrySet().iterator();
+ buddyIter.hasNext():wink:
+ {
+ if (buddyIter.next().getKey().equals(contact))
+ {
+ buddyIter.remove();
+ break;
+ }
+ }
     }

     /**
      * Returns an Iterator over all contacts, member of this
      * <tt>ContactGroup</tt>.
- *
+ *
      * @return a java.util.Iterator over all contacts inside this
- * <tt>ContactGroup</tt>. In case the group doesn't contain any
- * memebers it will return an empty iterator.
+ * <tt>ContactGroup</tt>. In case the group doesn't contain any
+ * members it will return an empty iterator.
      */
     public Iterator<Contact> contacts()
     {
@@ -368,7 +377,7 @@

     /**
- * Returns the icq contact encapsulating with the spcieified screen name or
+ * Returns the icq contact encapsulating with the specified screen name or
      * null if no such contact was found.
      *
      * @param screenName the screenName (or icq UIN) for the contact we're

Modified: trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java?view=diff&rev=5309&p1=trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java&p2=trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java&r1=5308&r2=5309

--- trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java (original)
+++ trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java 2009-04-30 10:41:06+0000
@@ -1657,9 +1657,16 @@
                  ((ContactGroupIcqImpl)parent).removeContact(srcContact);
                  theAwaitingAuthorizationGroup.addContact(srcContact);

- Object lock = new Object();
- synchronized(lock){
- try{ lock.wait(500); }catch(Exception e){}
+ try
+ {
+ Thread.sleep(500);
+ }
+ catch (InterruptedException ex)
+ {
+ /*
+ * I don't know why the exception is ignored, I just fixed
+ * an incorrect use of Object.wait(long).
+ */
                  }

                  fireSubscriptionMovedEvent(srcContact,

Modified: trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java?view=diff&rev=5309&p1=trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java&p2=trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java&r1=5308&r2=5309

--- trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java (original)
+++ trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java 2009-04-30 10:41:06+0000
@@ -14,6 +14,7 @@

import java.awt.*;
import java.awt.event.*;
+
import javax.swing.*;
import javax.swing.event.*;

@@ -243,15 +244,15 @@

     /**
      * Fills the Account ID, Identity File and Known Hosts File fields in this
- * panel with the data comming from the given protocolProvider.
+ * panel with the data coming from the given protocolProvider.
      *
      * @param protocolProvider The <tt>ProtocolProviderService</tt> to load the
      * data from.
      */
     public void loadAccount(ProtocolProviderService protocolProvider)
     {
- ProtocolProviderFactorySSH protocolProviderSSH =
- (ProtocolProviderFactorySSH)protocolProvider;
+ if (!(protocolProvider instanceof ProtocolProviderServiceSSHImpl))
+ throw new ClassCastException("protocolProvider");

         AccountID accountID = protocolProvider.getAccountID();

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: commits-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#2

Well... FindBugs did the discovery and, as we saw, I didn't fix it the
right way... But thank you for the appreciation.

···

On Thu, Apr 30, 2009 at 1:53 PM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Lubo,

That was a great catch. I had made a similar slip in
ContactGroupJabberImpl which is why the build was failing the last
several days. Wonder why they didn't catch this one.

Thanks for noticing!
Cheers
Emil

lubomir_m@dev.java.net wrote:

Author: lubomir_m
Date: 2009-04-30 10:41:06+0000
New Revision: 5309

Modified:
trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java
trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java
trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java

Log:
Fixes programming errors detected by FindBugs.

Modified: trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java?view=diff&rev=5309&p1=trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java&p2=trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java&r1=5308&r2=5309

--- trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java (original)
+++ trunk/src/net/java/sip/communicator/impl/protocol/icq/ContactGroupIcqImpl.java 2009-04-30 10:41:06+0000
@@ -12,7 +12,7 @@
import net.kano.joustsim.oscar.oscar.service.ssi.*;

/**
- * The ICQ implementation of the ContactGroup interface. Intances of this class
+ * The ICQ implementation of the ContactGroup interface. Instances of this class
* (contrary to <tt>RootContactGroupIcqImpl</tt>) may only contain buddies
* and cannot have sub groups. Note that instances of this class only use the
* corresponding joust sim source group for reading their names and only
@@ -33,7 +33,7 @@
* strings in the left column because screen names in AIM/ICQ are not case
* sensitive.
*/
- private Map<String, Contact> buddies
+ private final Map<String, Contact> buddies
= new Hashtable<String, Contact>();

 private boolean isResolved = false;

@@ -71,7 +71,7 @@
* to directly compare ( == ) instances of buddies we've stored and others
* that are returned by the framework.
* <p>
- * @param joustSimGroup the JoustSIM Group correspoinding to the group
+ * @param joustSimGroup the JoustSIM Group corresponding to the group
* @param groupMembers the group members that we should add to the group.
* @param ssclCallback a callback to the server stored contact list
* we're creating.
@@ -153,16 +153,25 @@
*/
void removeContact(ContactIcqImpl contact)
{
- buddies.remove(contact);
+ for (Iterator<Map.Entry<String, Contact>> buddyIter
+ = buddies.entrySet().iterator();
+ buddyIter.hasNext():wink:
+ {
+ if (buddyIter.next().getKey().equals(contact))
+ {
+ buddyIter.remove();
+ break;
+ }
+ }
}

 /\*\*
  \* Returns an Iterator over all contacts, member of this
  \* &lt;tt&gt;ContactGroup&lt;/tt&gt;\.

- *
+ *
* @return a java.util.Iterator over all contacts inside this
- * <tt>ContactGroup</tt>. In case the group doesn't contain any
- * memebers it will return an empty iterator.
+ * <tt>ContactGroup</tt>. In case the group doesn't contain any
+ * members it will return an empty iterator.
*/
public Iterator<Contact> contacts()
{
@@ -368,7 +377,7 @@

 /\*\*

- * Returns the icq contact encapsulating with the spcieified screen name or
+ * Returns the icq contact encapsulating with the specified screen name or
* null if no such contact was found.
*
* @param screenName the screenName (or icq UIN) for the contact we're

Modified: trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java?view=diff&rev=5309&p1=trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java&p2=trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java&r1=5308&r2=5309

--- trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java (original)
+++ trunk/src/net/java/sip/communicator/impl/protocol/icq/OperationSetPersistentPresenceIcqImpl.java 2009-04-30 10:41:06+0000
@@ -1657,9 +1657,16 @@
((ContactGroupIcqImpl)parent).removeContact(srcContact);
theAwaitingAuthorizationGroup.addContact(srcContact);

- Object lock = new Object();
- synchronized(lock){
- try{ lock.wait(500); }catch(Exception e){}
+ try
+ {
+ Thread.sleep(500);
+ }
+ catch (InterruptedException ex)
+ {
+ /*
+ * I don't know why the exception is ignored, I just fixed
+ * an incorrect use of Object.wait(long).
+ */
}

              fireSubscriptionMovedEvent\(srcContact,

Modified: trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java
Url: https://sip-communicator.dev.java.net/source/browse/sip-communicator/trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java?view=diff&rev=5309&p1=trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java&p2=trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java&r1=5308&r2=5309

--- trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java (original)
+++ trunk/src/net/java/sip/communicator/plugin/sshaccregwizz/FirstWizardPage.java 2009-04-30 10:41:06+0000
@@ -14,6 +14,7 @@

import java.awt.*;
import java.awt.event.*;
+
import javax.swing.*;
import javax.swing.event.*;

@@ -243,15 +244,15 @@

 /\*\*
  \* Fills the Account ID, Identity File and Known Hosts File fields in this

- * panel with the data comming from the given protocolProvider.
+ * panel with the data coming from the given protocolProvider.
*
* @param protocolProvider The <tt>ProtocolProviderService</tt> to load the
* data from.
*/
public void loadAccount(ProtocolProviderService protocolProvider)
{
- ProtocolProviderFactorySSH protocolProviderSSH =
- (ProtocolProviderFactorySSH)protocolProvider;
+ if (!(protocolProvider instanceof ProtocolProviderServiceSSHImpl))
+ throw new ClassCastException("protocolProvider");

     AccountID accountID = protocolProvider\.getAccountID\(\);

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: commits-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net