Current implementation of checkForEmptyProperty() isnt working

[jn0101]

I just spotted some code which can't work:


package de.kugihan.dictionaryformids.dataaccess;


public class DictionaryDataFile  {
...

   public static void initValues(boolean initDictionaryGenerationValues)
   {
...
      dictionaryAbbreviation = utilObj.getDictionaryPropertyString("dictionaryAbbreviation", true);
      checkForEmptyProperty(dictionaryAbbreviation);
        }


   // if property is provided but empty, then this is handled as if no property was provided:
   static void checkForEmptyProperty(String propertyName) {
      if (propertyName != null)
         if (propertyName.length() == 0)
            propertyName = null;
   }



If you'd want it to work you'd have to return a value, say like this:


   public static void initValues(boolean initDictionaryGenerationValues)
   {
...
      dictionaryAbbreviation = utilObj.getDictionaryPropertyString("dictionaryAbbreviation", true);
      dictionaryAbbreviation = checkForEmptyProperty(dictionaryAbbreviation);
        }


   // if property is provided but empty, then this is handled as if no property was provided:
   static String checkForEmptyProperty(String propertyName) {
      if (propertyName != null)
         if (propertyName.length() == 0)
            propertyName = null;
                return propertyName;
   }

Jacob

[Gert]

Wow - you are are a great reviewer !!

Maybe ... you would like to review and rework 'the big problem' in the Java ME implementation: class StringColourItem, see http://dictionarymid.sourceforge.net/forum/index.php?topic=145.msg725#msg725

Some specific points concerning SE models had been looked at, but we still search for someone who has the knowledge and time to check and if needed overhaul the class StringColourItem. So if you had some time ... !


Back to the error that you reported: I still need to look at that a little closer; also propertyName as parameter strange, cause it is not the property name but the value that is checked. I really need to check that.

Regards,
Gert

[jn0101]

Sorry, I don't have enough experience in J2ME coding.

I could do a lot, but to restructure and refactor anything, I need unit tests, to see if I break something.

With no unit tests its most often best to leave the code as is - you just risk breaking a lot of things without noticing and you would be using time during the following years, finding out about broken stuff by user feedback and cleaning up - and give bad user expeciences at the same time.

Jacob

[Gert]

Well, yes, we really should have documented unit tests. If possible automated unit tests. Until now, as far as I can see, we do not have any documented unit tests.

I really see this as an area where we should improve in the future.

Specifically, with regards to StringColourItem, I assume that including improvements will make things better than they are now, hmmmm, maybe StringColourItem even would be a good point to start with documented unit tests   

Regards,
Gert

P.S.: the problem about checkForEmptyProperty, I fixed that meanwhile, but I still wait with the commit to SVN until I have a few more updates done.