[sip-comm-dev] Strange code in CreateGroup class


#1

Hi,
the CreateGroup class is a thread, but it's run() method starts
another thread, is there a reason for that ?

Matthieu

···

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


#2

Hi Matthieu,

Hi,
the CreateGroup class is a thread, but it's run() method starts
another thread, is there a reason for that ?

Absolutely, we can not leave the poor CreateGroup thread alone:)

This is a nice one:)) Feel free to fix it.

Cheers,
Yana

···

On Apr 14, 2010, at 6:16 PM, Matthieu Casanova wrote:

Matthieu

---------------------------------------------------------------------
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


#3

Done,
I had another question, why not using a threadpool, I think it would
be very useful.
And a few things about coding style: do you think it would be a good
idea to make fields final when it is possible. Doing that make the
code easier to understand since we are sure that some field will never
change.
And also use interfaces instead of their class implementation ( have
fields like
private List list = new ArrayList()
instead of
private ArrayList list = new ArrayList()
for example)
And also avoid use of Vector and Hashtable when not necessary since
those classes are synchronized even if we don't need that.
Of course if you agree I don't suggest to replace everything
immediately but it can be done sometimes when modifying a class or
reviewing code ?

Matthieu

···

2010/4/14 Yana Stamcheva <yana@sip-communicator.org>:

Hi Matthieu,

On Apr 14, 2010, at 6:16 PM, Matthieu Casanova wrote:

Hi,
the CreateGroup class is a thread, but it's run() method starts
another thread, is there a reason for that ?

Absolutely, we can not leave the poor CreateGroup thread alone:)

This is a nice one:)) Feel free to fix it.

Cheers,
Yana

Matthieu

---------------------------------------------------------------------
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

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


#4

Hi Matthieu,

Done,
I had another question, why not using a threadpool, I think it would
be very useful.

Are you talking about the create group operation here or in general ? Could you be more precise where you think it would be useful to use a threadpool ?

And a few things about coding style: do you think it would be a good
idea to make fields final when it is possible. Doing that make the
code easier to understand since we are sure that some field will never
change.
And also use interfaces instead of their class implementation ( have
fields like
private List list = new ArrayList()
instead of
private ArrayList list = new ArrayList()
for example)
And also avoid use of Vector and Hashtable when not necessary since
those classes are synchronized even if we don't need that.
Of course if you agree I don't suggest to replace everything
immediately but it can be done sometimes when modifying a class or
reviewing code ?

Absolutely agree. I, personally, am trying lately to change these whenever I see them. Feel free to modify such code.

Cheers,
Yana

···

On Apr 14, 2010, at 9:31 PM, Matthieu Casanova wrote:

Matthieu

2010/4/14 Yana Stamcheva <yana@sip-communicator.org>:

Hi Matthieu,

On Apr 14, 2010, at 6:16 PM, Matthieu Casanova wrote:

Hi,
the CreateGroup class is a thread, but it's run() method starts
another thread, is there a reason for that ?

Absolutely, we can not leave the poor CreateGroup thread alone:)

This is a nice one:)) Feel free to fix it.

Cheers,
Yana

Matthieu

---------------------------------------------------------------------
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

---------------------------------------------------------------------
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


#5

Hi Yana,

Hi Matthieu,

Done,
I had another question, why not using a threadpool, I think it would
be very useful.

Are you talking about the create group operation here or in general ? Could you be more precise where you think it would be useful to use a threadpool ?

I'm talking about a threadpool that would be used everywhere in
sip-communicator. Instead of creating threads everytime that is a
heavy task for the system, threads would be created before and always
ready to work.
Basically instead of having

Thread thread = new Thread()
{
  public void run()
  {
    // do some work
  }
};
thread.start();

We would have
Runnable runnable = new Runnable()
{
  public void run()
  {
    // do some work
  }
};
Executor executor = ... get the executor service

executor.execute(runnable);
and the executor would run the task as soon as possible on the first
available thread.

And a few things about coding style: do you think it would be a good
idea to make fields final when it is possible. Doing that make the
code easier to understand since we are sure that some field will never
change.
And also use interfaces instead of their class implementation ( have
fields like
private List list = new ArrayList()
instead of
private ArrayList list = new ArrayList()
for example)
And also avoid use of Vector and Hashtable when not necessary since
those classes are synchronized even if we don't need that.
Of course if you agree I don't suggest to replace everything
immediately but it can be done sometimes when modifying a class or
reviewing code ?

Absolutely agree. I, personally, am trying lately to change these whenever I see them. Feel free to modify such code.

Ok perfect I'll do that

Matthieu

···

2010/4/15 Yana Stamcheva <yana@sip-communicator.org>:

On Apr 14, 2010, at 9:31 PM, Matthieu Casanova wrote:

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