Fix for #1190 Serial monitor colours not right #1191
Conversation
| newSerial.registerService(); | ||
| SerialListener theListener = new SerialListener(this, colorindex); | ||
| newSerial.addListener(theListener); | ||
| String newLine=System.getProperty("line.separator");//$NON-NLS-1$ |
There was a problem hiding this comment.
To me newLine is far more readable then NL. So I'm not sure what the added value is of using NL
There was a problem hiding this comment.
I should have named the field NEW_LINE. But it's an unrelated change so I'll just undo it.
There was a problem hiding this comment.
As you may have noticed I like verbose code ;-) NEW_LINE is ok for me
| private static SerialMonitor instance = null; | ||
| | ||
| public static SerialMonitor getSerialMonitor() { | ||
| public static synchronized SerialMonitor getSerialMonitor() { |
There was a problem hiding this comment.
I assume this means the comment " Must be run from the ui thread" can be removed from derialconnection.add and serial connection.remove
There was a problem hiding this comment.
I just did that because I figured any singleton's get... method should be sync'd.
There was a problem hiding this comment.
I'm not sure what a singleton is. I just want to keep the doc up to date as possible
There was a problem hiding this comment.
SerialMonitor implements the singleton pattern in a textbook fashion, other than not protecting the getSerialMonitor method. You have a single instance held in a private static field, and you get that instance via the getSerialMonitor method.
The normal practice is to protect against two threads running getXxx at the same time by synchronizing it. Then any number of threads can call simultaneously it and you'll still only ever have one instance created because only one thread can hit the == null test in the method at a time.
So that comment probably isn't necessary if the method is sync'd, as long as there is no other reason it should only be called from the UI thread.
There was a problem hiding this comment.
OK, not quite textbook singleton. The constructor should be private, but it can't be because Eclipse wants to call it via introspection.
But still, the idea is the same. You only want one of them.
Actually, if Eclipse is creating the instance via the constructor, does the null check in getSerialMonitor ever trigger?
| Thanks by the way. |
2 participants
This needs to be fixed before I can redo the timestamp code in SerialMonitor.