DictionaryForMids Forum

Dictionaries => General discussions => Topic started by: jn0101 on 29. April 2010, 20:14:53

Title: Current implementation of checkForEmptyProperty() isnt working
Post by: jn0101 on 29. April 2010, 20:14:53
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
Title: Re: Current implementation of checkForEmptyProperty() isnt working
Post by: Gert on 30. April 2010, 21:18:53
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
Title: Re: Current implementation of checkForEmptyProperty() isnt working
Post by: jn0101 on 05. May 2010, 10:04:33
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
Title: Re: Current implementation of checkForEmptyProperty() isnt working
Post by: Gert on 05. May 2010, 10:57:07
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.