1

In my application, I require dealing with multiple font files. So, instead of creating new instance every time, I implemented Singleton to get Typeface like this:

public class FontSingleton {

    private static FontSingleton instance;
    private static Typeface typeface;

    private FontSingleton(){
        //private constructor to avoid direct creation of objects
    }

    public static FontSingleton getInstance(Context context,String fontFileName){
        if(instance==null){
            instance = new FontSingleton();
            typeface = Typeface.createFromAsset(context.getResources().getAssets(), fontFileName);
        }
        return instance;
    }

    public Typeface getTypeFace(){
        return typeface;
    }
}

Now, I can get typeface like this:

FontSingleton.getInstance(mContext,"font1.otf").getTypeFace();

Is is the correct way to deal with memory leak and implementing Singleton? I am a newbie in design patterns and Android. Can anyone guide me to correct way?

0

3 Answers 3

3

There is no use making the FontSingleton a Singleton, cause what you really need is caching Typeface objects, not the class that creates them. Therefore you can make your FontSingleton (which is not a Singleton anymore, so let's call it FontHelper) a class with no instances, and let it store a Map of Typefaces. Those will be created in a lazy fashion: if there's no Typeface for a font name - create it and store it in the Map, otherwise reuse an existing instance. Here's the code:

public class FontHelper {

    private static final Map<String, Typeface> TYPEFACES = new HashMap<>();

    public static Typeface get(Context context,String fontFileName){
        Typeface typeface = TYPEFACES.get(fontFileName);
        if(typeface == null){
            typeface = Typeface.createFromAsset(context.getResources().getAssets(), fontFileName);
            TYPEFACES.put(fontFileName, typeface);
        }
        return typeface;
    }
}

Sorry if the code doesn't work as is, but hopefully the idea is clear.

1

This is over-complicated and incorrect if I see correctly (the instance will always keep the first typeface it has been given since it won't be called another time, due to the instance already existing).

Why not implement something like this:

public class FontHelper {
    public static Typeface get(Context context, String fontFilename) {
        return Typeface.createFromAsset(context.getResources().getAssets(), fontFileName);
    }
}

Then you just call:

FontHelper.get(mContext,"font1.otf")

3
  • 3
    I would cache the typefaces in a static HashMap as well.
    – Karakuri
    Jul 11, 2015 at 18:15
  • 1
    With this approach a new Typeface will be created every time the get() method is called, and retrieving assets is a pretty expensive operation: this code will be very slow.
    – Egor
    Jul 11, 2015 at 18:15
  • @Karakuri I agree. This was just from the top of my head. Jul 11, 2015 at 18:25
1

Is is the correct way to deal with memory leak and implementing Singleton?

getInstance() should be synchronized, after you mark it as synchronized, then, for what concerns the pattern's implementation it is correct. Still I think it doesn't suit your need well. For instance you can't create a new TypeFace object. The way you wrote it you are stuck using font1.otf. If you want that, than you don't need to provide the fontname, as parameter to getInstance().

As alternative solution, you should take in consideration the possibility to subclass TextView, providing a custom xml attribute through which you can specify the font you want to use.

5
  • thanks, but "you don't need to provide the fontname, as parameter to getInstance()". then how can I do it? Jul 11, 2015 at 18:11
  • but as I said I need to deal with multiple fonts. Should I have multiple methods to get instance? Jul 11, 2015 at 18:13
  • the way you wrote it, you will have always only one type of TypeFace. What you probably want is a Factory and not a Singleton, but if I were in you I would go for the subclass solution
    – Blackbelt
    Jul 11, 2015 at 18:15
  • 2
    Egor's solution does what you probably had in mind with the singleton. You can find here TextView's subclass. I didn't test it, so you'll probably have to fix some minor thing. Here is a gist with the code
    – Blackbelt
    Jul 11, 2015 at 18:32
  • @Blackbelt, That's a pretty handy solution, but it doesn't solve the problem of caching Typeface instances. A new Typeface will be created for each TextView instance.
    – Egor
    Jul 11, 2015 at 20:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.