I'll keep it in mind not to do pointless lock changes. Doing a synchronized(someObject) is perfectly fine, too.
On 28 April 2014 22:21, Ralph Goers <rgo...@apache.org> wrote: > I think you are still missing the point. Whether I agree with the > arguments or not, I still don't think it is worth the time to change stuff > like this that is already working just because it is considered a better > style. You have a better argument with the synchronization stuff because > it could possibly perform better (or not - there seems to be some > discussion on that), but I am not in favor of making changes just because > someone feels like redecorating the room. That said, you already made the > change and I don't think it is worth the effort to change it back either. > > Ralph > > On Apr 28, 2014, at 7:19 PM, Matt Sicker <boa...@gmail.com> wrote: > > Here, I found a debate over what I was trying to convey: > > > http://stackoverflow.com/questions/541487/implements-runnable-vs-extends-thread > > > On 28 April 2014 12:22, Ralph Goers <ralph.go...@dslextreme.com> wrote: > >> Preferring Executors to Threads really has nothing to do with the change >> below since there isn’t an Executor in sight in the old or the new. If you >> want a pool of Threads then item 68 applies. For a single thread creating a >> Thread with a run method is OK. So is the way you did it - it is just more >> verbose. >> >> Ralph >> >> >> On Apr 28, 2014, at 9:33 AM, Matt Sicker <boa...@gmail.com> wrote: >> >> It's based on Item 68 from Effective Java (prefer executors and tasks to >> threads), and those use Runnable and Callable<T> instead of Thread. Plus >> it's simpler to implement Runnable and construct a Thread from it if >> necessary. Overall, it's better to use the Executors class to spawn off >> threads. >> >> >> On 27 April 2014 23:22, Ralph Goers <ralph.go...@dslextreme.com> wrote: >> >>> I wondered the same thing myself. I think Matt stated he preferred >>> Runnable to extending Thread in the past, but I really don’t like changes >>> like this just for the sake of someone’s personal preference. While we do >>> have sort of an unwritten list of coding guidelines this one isn’t on that >>> list (at least, not yet). >>> >>> Ralph >>> >>> >>> On Apr 27, 2014, at 8:22 PM, Remko Popma <remko.po...@gmail.com> wrote: >>> >>> > Hi Matt, >>> > I don't mind this change, but why do you think this is better? This is >>> all private internal & the previous code was shorter... >>> > >>> > Remko >>> > >>> > Sent from my iPhone >>> > >>> >> On 2014/04/28, at 3:31, mattsic...@apache.org wrote: >>> >> >>> >> Author: mattsicker >>> >> Date: Sun Apr 27 18:31:20 2014 >>> >> New Revision: 1590447 >>> >> >>> >> URL: http://svn.apache.org/r1590447 >>> >> Log: >>> >> Convert anonymous thread to runnable. >>> >> >>> >> Modified: >>> >> >>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java >>> >> >>> >> Modified: >>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java >>> >> URL: >>> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java?rev=1590447&r1=1590446&r2=1590447&view=diff >>> >> >>> ============================================================================== >>> >> --- >>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java >>> (original) >>> >> +++ >>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java >>> Sun Apr 27 18:31:20 2014 >>> >> @@ -31,20 +31,20 @@ public final class CachedClock implement >>> >> private static CachedClock instance = new CachedClock(); >>> >> private volatile long millis = System.currentTimeMillis(); >>> >> private volatile short count = 0; >>> >> - private final Thread updater = new Thread("Clock Updater >>> Thread") { >>> >> - @Override >>> >> - public void run() { >>> >> - while (true) { >>> >> - final long time = System.currentTimeMillis(); >>> >> - millis = time; >>> >> - >>> >> - // avoid explicit dependency on sun.misc.Util >>> >> - LockSupport.parkNanos(1000 * 1000); >>> >> - } >>> >> - } >>> >> - }; >>> >> >>> >> private CachedClock() { >>> >> + final Thread updater = new Thread(new Runnable() { >>> >> + @Override >>> >> + public void run() { >>> >> + while (true) { >>> >> + final long time = System.currentTimeMillis(); >>> >> + millis = time; >>> >> + >>> >> + // avoid explicit dependency on sun.misc.Util >>> >> + LockSupport.parkNanos(1000 * 1000); >>> >> + } >>> >> + } >>> >> + }, "Clock Updater Thread"); >>> >> updater.setDaemon(true); >>> >> updater.start(); >>> >> } >>> >> >>> >> >>> > >>> > --------------------------------------------------------------------- >>> > To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org >>> > For additional commands, e-mail: log4j-dev-h...@logging.apache.org >>> > >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org >>> For additional commands, e-mail: log4j-dev-h...@logging.apache.org >>> >>> >> >> >> -- >> Matt Sicker <boa...@gmail.com> >> >> >> > > > -- > Matt Sicker <boa...@gmail.com> > > -- Matt Sicker <boa...@gmail.com>
