[izpack-devel] Patch: InstallerGroupPanel reading descriptionfromvariable not working

[Markus Schlegel]

Sorry, this was a reminder for me to mention it when I send in the
patch.

This is what I had to mention:
I guess that it never makes sense to display a technical Keyname to the
user. This is just a debugging aid.
I do not have a problem when such a keyname is displayed as a debugging
aid. The "problem" in my opinion is, that "LocalDatabase.getString(key)"
should return null instead of the key (in both cases, the user of this
method has to check the returnvalue, but it's cleaner and cheaper to
check against null than against a String...). 

A Method "getSomething(String key) with argument key in Java is commonly
interpreted as a lookup method. Of corse the developer wrote a javadoc
comment that states, that this method retuns the key if the value could
not be found. But unfortunately he missed to catch every case where a
key is not the desired result when he refactored IzPanel.getI18N..(key)
to LocalDatabase.getString(key)...

Also I understand that you have very rare spare time (as I do), such
that there is no time to think about such minor things.

Here is the Patch without the comment:

Index:
C:/deron/tools/IzPack_TRUNK/src/lib/com/izforge/izpack/panels/Installati
onGroupPanel.java
===================================================================
---
C:/deron/tools/IzPack_TRUNK/src/lib/com/izforge/izpack/panels/Installati
onGroupPanel.java	(revision 1665)
+++
C:/deron/tools/IzPack_TRUNK/src/lib/com/izforge/izpack/panels/Installati
onGroupPanel.java	(working copy)
@@ -383,7 +383,7 @@
             else
                 description = html;
         }
-        if (description == null)
+        if (description == null || key.equalsIgnoreCase(description)) 
             description = idata.getVariable(key);
         if (description == null)
             description = group + " installation";



________________________________

From: izpack-devel-bounces at lists.berlios.de
[mailto:izpack-devel-bounces at lists.berlios.de] On Behalf Of Bartz, Klaus
Sent: Monday, December 11, 2006 4:33 PM
To: izpack-devel at lists.berlios.de
Subject: Re: [izpack-devel] Patch: InstallerGroupPanel reading
descriptionfromvariable not working


Hi,
until jet I have not worked with the InstallerGroupPanel, therefore I do
not know whether the
key name is not the right if the langpack do not contain a value for the
key or not. If this
behavior is wrong, it should be fixed in the class
InstallationGroupPanel; but please 
without the wrong comment. This behavior is written in the JavaDoc...
 
In principle it is a good idea to return the key symbol if the value not
found. Julien has
made it some years ago to prevent the installation to fail with a null
pointer exception.
 
Cheers
 
Klaus
 

	-----Original Message-----
	From: izpack-devel-bounces at lists.berlios.de
[mailto:izpack-devel-bounces at lists.berlios.de]On Behalf Of Markus
Schlegel
	Sent: Monday, December 11, 2006 3:16 PM
	To: izpack-devel at lists.berlios.de
	Subject: [izpack-devel] Patch: InstallerGroupPanel reading
description fromvariable not working
	
	
	Due to the fact, that LocalDatabase.getString(key) returns the
key if no such value could be found,  the now patched line never get's a
chance to read the textfrom the defined variable.
	This is now working again.
	 
	Index:
C:/deron/tools/IzPack_TRUNK/src/lib/com/izforge/izpack/panels/Installati
onGroupPanel.java
	
===================================================================
	---
C:/deron/tools/IzPack_TRUNK/src/lib/com/izforge/izpack/panels/Installati
onGroupPanel.java (revision 1665)
	+++
C:/deron/tools/IzPack_TRUNK/src/lib/com/izforge/izpack/panels/Installati
onGroupPanel.java (working copy)
	@@ -383,7 +383,7 @@
	             else
	                 description = html;
	         }
	-        if (description == null)
	+        if (description == null ||
key.equalsIgnoreCase(description)) //not a good idea, to return key if
value not found...
	             description = idata.getVariable(key);
	         if (description == null)
	             description = group + " installation";