Thanks, Miguel. This is excellent feedback.
Miguel wrote:
Bob,
Some brief feedback about PropertyManager.java ...
* methods in here should not be public. All public entry points are in
viewer. viewer calls the the routines in PropertyManager. They are in the
same package so they should not be public in PropertyManager.
I knew that. I think I was experimenting and forgot to remove it. It's
out now on my end.
* I am not sure that 'synchronized' goes there. But there are lots of race
conditions in the code, so just leave it for now.
I'm almost certainly my crashes were due to simultaneous entry into
statusManager. When you have different threads writing to that and
reading from it (which flushes as it reads), you're just asking for
trouble, I think. But they almost certainly are overkill. I dont't
figure these particular routines need to be super efficient. Not like
rendering.
* You need to understand something about String comparisions:
if (returnType == "String") return info.toString();
This type of comparison works *only* if returnType is an 'interned'
string. An 'interned' string is guaranteed to be from a special string
pool, so when comparing interned strings you can just compare the object
pointers without looking at the string contents.
Are you saying that exact comparison, of a varible with a static string,
could be prone to this problem, or only when two variable strings are
involved?
Now, all string constants are guaranteed to be interned, so that I why I
use this idiom for lots of internal dispatching.
But, you need to be careful about when/where you use this. If you ever
expect to pass this in as a parameter from JavaScript, then it probably
will *not* be interned. In addition, naive/unsuspecting Java programmers
will certainly get bitten by this when they make use of the API.
The rule is ... inside an external API entry point you must intern any
parameters (using param = param.intern(); or equiv) if you are going to be
making these types of == comparisons with strings.
* We should probably use the '==' string object compare mechanism for the
propertyName. That will replace the equalsIgnoreCase calls. These property
names will be case sensitive.
makes sense to me. I in fact switched to equalsIgnoreCase() specifically
because I did get caught by the interning business. I'll read up on
interning.
* I expect the number of properties to grow to at least one hundred.
Therefore, over time we will evolve some clean mechanism to dispatch based
upon propertyName.
whew. You think? Can you save method pointers in properties, then just
"get()" the pointers and run their associated functions?
* Regarding variables ... within the bodies of functions/methods,
variables should be declared before/as they are used, not at the beginning
of the block. This is especially true if the loop is really a for loop. I
suggest the following transformation of your code:
String simpleReplace(String str, String strFrom, String strTo) {
String sout = "";
int ipt;
int ipt0 = 0;
int lfrom = strFrom.length();
if (str == null || lfrom == 0) return str;
while ((ipt = str.indexOf(strFrom, ipt0)) >=0) {
sout = str.substring(ipt0,ipt) + strTo; // <--- BUG HERE ?
ipt0 = ipt + lfrom;
}
sout = sout + str.substring(ipt0,str.length());
return sout;
}
yeah, sout +=
-->
String simpleReplace(String str, String strFrom, String strTo) {
int fromLength = strFrom.length();
if (str == null || fromLength == 0)
return str;
int ipt;
int ipt0;
String sout = "";
for (ipt0 = 0;
ipt = str.indexOf(strFrom, ipt0);
ipt0 = ipt + fromLength)
sout += str.substring(ipt0, ipt) + strTo;
sout += str.substring(ipt0, str.length());
return sout;
}
declaring them up front is a carryover for me from Visual Basic
EXPLICIT. But I can adjust. Still a bug there -- your int = definition
isn't boolean, I think. I like the while better, personally. I see I
forgot to test that, though. My bad.
Is there any efficiency to be gained in .concat() relative to += ?
As a minor note, Java provides a StringBuffer data structure that may be
more efficient when you are catting lots of things together.
so that's what that's for.
* getProperty should not take a 'returnType'
How does that work? Then it can take whatever type is being returned and
not turn that type into an Object? I'll look into that as well.
This is about all I can do this week on this. What are you working on?
Bob
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
_______________________________________________
Jmol-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/jmol-developers
