Bug 49025 - Util.bind(String, String[]) can be optimized a little bit
Summary: Util.bind(String, String[]) can be optimized a little bit
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows 2000
: P3 enhancement (vote)
Target Milestone: 3.0 M9   Edit
Assignee: Frederic Fusier CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-12-17 10:33 EST by Frederic Fusier CLA
Modified: 2004-05-18 11:46 EDT (History)
1 user (show)

See Also:


Attachments
Uses the char[] instead of Strings (2.87 KB, patch)
2004-04-01 09:38 EST, Michael Fraenkel CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frederic Fusier CLA 2003-12-17 10:33:20 EST
While verifying bug 45589, I had a look on merged method and I think that 
org.eclipse.jdt.internal.core.util.Util.bind(String, String[]) method can be 
optimized a little bit.

After getting string from bundle we remove all double quotes using 
CharOperation method which returns a char array. Then, we build a new string 
from this array and scan it to replace all {x} by given parameters.

In case of parameters array is not empty, it would be more efficient not to 
instanciate a new string but parse directly the array of chars. We would append 
in StringBuffer using append(char[], int, int) method which then would 
completely avoid any extra strings instanciation...
Comment 1 Michael Fraenkel CLA 2004-04-01 09:38:14 EST
Created attachment 9093 [details]
Uses the char[] instead of Strings

I noticed two things while I did this patch.
1. The original code has a bug (or I think it does) when something like {id} is
being coverted and id is not found in the bundle.  The result is id}.  My
version will display id.
2. Util.bind appears in two different packages:
org.eclipse.jdt.internal.compiler.util and org.eclise.jdt.internal.core.util.
It might be advisible to have a new method which takes the ResourceBundle as an
argument so you can remove one version.
Comment 2 Frederic Fusier CLA 2004-04-02 11:50:21 EST
Thanks Michael for your patch which looks good :)

One question:
I do not really understand the bug you want to warn us about. Do you mean a key 
in properties files like:
  {key with braces} = xxxxxxxxxxxxxxxxxxxxx
or:
   key = some {braces} with no numbers...
or something else?
Comment 3 Michael Fraenkel CLA 2004-04-04 19:38:27 EDT
The parsing handles alternative expansions inside a {} pair.  In the case when 
the contents is neither an integer or another key, it displays the value.  
However, the value is displayed with the ending }.  For example, if you had 
{no match}, the result would be "no match}".  With the change I made, it will 
print "no match".  A possible off-by-one issue.
Comment 4 Michael Fraenkel CLA 2004-04-05 21:13:35 EDT
I realized that the best option may be to leave the {} in the text.  My fix 
was to remove the { } which hides the potential problem.  Guess the question 
is what should {non-numeric/resource-bundle key} display.
Comment 5 Frederic Fusier CLA 2004-04-26 06:32:46 EDT
Fixed.

Patch applied to:
org.eclipse.jdt.core.compiler.internal.util.Util.bind[String, String[])
org.eclipse.jdt.internal.core.util.Util.bind[String, String[])
org.eclipse.jdt.core.compiler.problem.DefaultProblemFactory.getLocalizedMessage
(int, String[])

[jdt-core-internal]
Also, put back 1.5 stream changes for DefaultProblemFactory...

Local test cases (ie. not public) have been written, passed and released in 
org.eclipse.jdt.core.tests.javadoc project to verify that these changes has no 
impact for messages of following properties files:
org/eclipse/jdt/core/compiler/internal/util/messages.properties
org/eclipse/jdt/internal/core/util/messages.properties
org/eclipse/jdt/core/compiler/problem/messages.properties

No specific test cases added. Assume that existing tests cases of 
RunAllJDTCoreTests are sufficient to insure it works well...
Comment 6 Olivier Thomann CLA 2004-05-18 11:46:40 EDT
Verified that the patch is in 200405180816