design patterns how to avoid instanceOf when using List

Question

Imagine you have a menu with dishes each dish should be available in multiple languages (French , English , Arabic , ...). The Dish class contains a list with Language type objects.

class Dish { 
     List<Language> languages
     void addLanguage(Language lg){...}
}
class Language { getDescription(){}}

class French extends Language{}

class Menu {List<Dish> dishes }

How do I avoid using instance of when wanting a description of a specific language for that dish?

Should I define for each language a get method in the dish class:getFrench(), getArabic(),..?

Or should I keep it in list and check for instance of French by looping the list and then call getDescription() on this list object<language> ?

Or is there a way for a more polymorphic approach?

what is the return type of `getDescription()`, is it `String`? — Sharon Ben Asher, Sep 03 '18 at 08:08
For your specific case, I would use Resources instead of a `List`. But if you want to keep this logic, a `Map` would be better, with a default value just in case. — AxelH, Sep 03 '18 at 08:10
the solution proposed by AxelH raises the question what does `Language` hold? is it only the description or are there additional properties — Sharon Ben Asher, Sep 03 '18 at 08:14
To avoid special cases in general, you have to define a common interface. That interface announces e.g. the `getLanguage` method. All concrete languages must implement this interface. Take a look at the Open/Closed principle: http://joelabrahamsson.com/a-simple-example-of-the-openclosed-principle/ — Rene Knop, Sep 03 '18 at 08:16

Eran · Accepted Answer · 2018-09-03T08:42:17.360

7

I don't think it's a good idea to create a separate class for each language. After all, all these classes will have the exact same methods.

I'd use a single Language class, and inside the Dish class I'll keep a Map<Locale,Description> (I'd rename the Language class to something less confusing such as Description, since it doesn't represent a language, it represents a description in some language).

Now you can have a

Description getDescription(Locale locale)

method in your Dish class, which will return the description of the dish in the language of the passed Locale.

You should prefer using standard JDK classes such as java.util.Locale instead of custom classes when possible.

After considering the comments, and agreeing to some of them, I suggest removing the Map to the Description class.

class Dish 
{ 
      Description description = new Description ();

      void addDescription(Locale locale, String text)
      {
          description.addText(locale,text);
      }

      String getDescription(Locale locale) 
      {
          return description.getText(locale);
      }
 }

class Description 
{
    Map<Locale,String> descriptions = new HashMap<>();

    public void addText(Locale locale,String text) 
    {
        descriptions.put(locale,text);
    }

    public void getText(Locale locale) 
    {
        return descriptions.get(locale);
    }
}

You should also note that searching the language specific description in a Map is more efficient than searching for it in a List (O(1) lookup time in a HashMap vs. O(n) lookup time in a List).

edited Sep 03 '18 at 08:42

answered Sep 03 '18 at 08:14

Eran

387,369
54
702
768

You have added a responsibility to the Dish to maintain localizations which makes it more difficult to test and maintain (also the localization management code is less reusable when it is in the Dish-class). To preserve the single responsibility principle, I would remove all references to languages from the Menu and Dish and move them to the Description. E.g. you get one description from the Dish and that description contains all available localizations and code to maintain them. – Torben Sep 03 '18 at 08:24
@Torben Do you suggest a central authority for all translations? Like a wrapper class that is responsible for translations? – Thiyagu Sep 03 '18 at 08:29
2

@Torben good point. I agree it would probably be better if Dish doesn't have to maintain the localization Map. This can be avoided by putting the localization in the Description class. Dish will contain a single Description instance, and will call description.getDescription(locale) to obtain a locale specific description. I still prefer not to create a separate class for each Language. – Eran Sep 03 '18 at 08:29
@Torben So a bit more like the answer from Rene Link ? – majestic Sep 03 '18 at 08:32
Or what if it is part of the parent class? The parent class has `getDescription(Locale locale)` and an abstract method `getDescription()` that the child overrides? I guess this can be useful if there are more subclasses (the child classes provides description in default language and translation is taken care elsewhere) – Thiyagu Sep 03 '18 at 08:34
@majestic René's solution still asks for a description in a language from Dish. – Torben Sep 03 '18 at 12:40

René Link · Answer 2 · 2018-09-03T08:36:16.953

I think a Language doesn't have a menu Description. I would design it in this way.

 class Dish { 
       List<Description> descriptions;
       void addDescription(Description description){...}

       Description getDescription(Language language){
           // iterate over descriptions and select the appropriate
           // or use a Map<Language, Description>
           Optional<Description> applicableDescription =
                          descriptions.stream()
                          .filter(d -> d.getLanguage().isApplicable(language)).findFirst();
           return applicableDescription.orElseGet(this::getDefaultDescription);
       }

       private Description getDefaultDescription(){
           return descriptions.isEmpty() ? null : descriptions.get(0); 
       }
 }


 class Description {
     Language language;
     String shortDescription;
     String title;
     // and so on... 

     public boolean isApplicable(Language language){
         return this.language.equals(language);
         // This logic can be more sophisticated. E.g. If an Italian
         // wants to get a menu description, but no italian translation is
         // available, it might be useful to choose spanish. But then it
         // would be better to return a similarity, like 0.1 to 1.0, instead of true/false.
     }
 }

 class Language {
      String name; // like en, de, it, fr, etc. Maybe an enum is better or just using
                   // java.util.Locale
 }

AxelH · Answer 3 · 2018-09-03T08:36:02.983

To propose a different approach about multi language solution, I would suggest to take a look at ResourceBundle. It allows to get values based on the locale.

For instance, let's define to file :

The first for english :

data_en.properties
hello=Hello World

And for the french version :

data_fr.properties
hello=Bonjour le monde

Now, let's load the resources and print the value :

//Locale.setDefault(Locale.FRENCH);
System.out.println(Locale.getDefault());
ResourceBundle rb = ResourceBundle.getBundle("data");
System.out.println(rb.getString("hello"));

It will print :

en_US
Hello World

And if I un-comment the first line to emulate a French locale :

fr
Bonjour le monde

This can be used to get different translation. Note that I never used it for data, only for GUI (or fixed values) but based on this question, this should be quite possible to adapt the solution to use a database.

How to load resource bundle messages from DB in Java?

The advantage ? you don't have to load every translations in memory, only the one for the Locale you are running this, so only the one you really need.

design patterns how to avoid instanceOf when using List

3 Answers3