CODEWITHSUNDEEP
javaandroidoop
uTubeFan
uTubeFan

Reputation: 6794

Is having to pass Context to most classes a sign of bad design?

Android is designed in such a way that in order for a method to read a resource, it must have a access to a Context.

Since most of the classes in my application rely on the flexibility of string resources (e.g. changing language without having to change code, etc.), my simple solution was to pass a Context in their constructor, to provide resource access to each such class.

It doesn't make sense for me to only pass a string in the constructor, because the class needs the flexibility of access to a varying number of strings.

So, to simplify things, I only pass Context, and whenever a string resource is needed, I just use Context.getString().

Is this a sign of bad design?

Is there a better way to accomplish this?

Upvotes: 7

Views: 3264

Answers (4)

Candleman
Candleman

Reputation: 11

I don't know if it's a bad practice to use it this way, but I did time comparison that took to load 200 light-weight object into Array-list with one of the Constructors taking Context as parameter, and other one getting the required context through static method like @Eric Farr suggested. End result was that accessing context through static method instead of passing to constructor was always 5-25% faster..

@Override
public void surfaceCreated(SurfaceHolder surfaceHolder) {
    initBlocks(map); // takes context through static method
    blocks.clear();
    initBlocks(getContext(), map); // pass context down to every block object
    mainthread = new GameThread(getHolder(), this);
    mainthread.setRunning(true);
    mainthread.start();
}


I/art: Background sticky concurrent mark sweep GC freed 1070(51KB) AllocSpace 
objects, 44(3MB) LOS objects, 27% free, 8MB/11MB, paused 27.450ms total 132.688ms
I/art: Background partial concurrent mark sweep GC freed 1638(66KB) AllocSpace 
objects, 36(3MB) LOS objects, 29% free, 9MB/13MB, paused 13.982ms total 184.900ms
I/art: Background sticky concurrent mark sweep GC freed 916(44KB) AllocSpace objects, 
37(3MB) LOS objects, 16% free, 11MB/13MB, paused 17.710ms total 127.963ms
**I/System.out: TIME IT TOOK TO PROCESS >> 2254155160**
I/art: Background partial concurrent mark sweep GC freed 1780(77KB) AllocSpace 
objects, 208(8MB) LOS objects, 38% free, 6MB/10MB, paused 22.850ms total 193.625ms
I/art: WaitForGcToComplete blocked for 5.262ms for cause Background
I/art: Background sticky concurrent mark sweep GC freed 959(46KB) AllocSpace objects, 
39(3MB) LOS objects, 24% free, 7MB/10MB, paused 15.160ms total 101.055ms
I/art: Background partial concurrent mark sweep GC freed 1873(71KB) AllocSpace 
objects, 32(2MB) LOS objects, 32% free, 8MB/12MB, paused 17.253ms total 154.302ms
I/art: Background sticky concurrent mark sweep GC freed 888(42KB) AllocSpace objects, 
36(3MB) LOS objects, 17% free, 10MB/12MB, paused 42.191ms total 179.613ms
I/art: Background partial concurrent mark sweep GC freed 1243(50KB) AllocSpace 
objects, 30(2MB) LOS objects, 27% free, 10MB/14MB, paused 29.423ms total 283.968ms
**I/System.out: TIME IT TOOK TO PROCESS >> 3079467060**
I/art: Background sticky concurrent mark sweep GC freed 343(16KB) AllocSpace objects, 
14(1232KB) LOS objects, 0% free, 21MB/21MB, paused 20.481ms total 162.576ms
I/art: Background partial concurrent mark sweep GC freed 396(13KB) AllocSpace 
objects, 4(280KB) LOS objects, 15% free, 21MB/25MB, paused 21.971ms total 243.764ms
I/art: Background sticky concurrent mark sweep GC freed 471(166KB) AllocSpace 
objects, 0(0B) LOS objects, 0% free, 37MB/37MB, paused 26.350ms total 133.655ms
I/art: Background partial concurrent mark sweep GC freed 356(10KB) AllocSpace 
objects, 1(10MB) LOS objects, 12% free, 27MB/31MB, paused 41.909ms total 299.517ms

I don't know about you, but I'll take an increase in performance (y).

Upvotes: 1

Eric Farr
Eric Farr

Reputation: 2713

This is one of the rare occasions where a globally accessible singleton class may be better than passing the Context to every single class.

I would consider creating a singleton for localization, then use the Context inside of there (unless you need other aspects of the Context all over the place).

Of course, this is a matter of taste and preference. YMMV

Upvotes: 4

ggurov
ggurov

Reputation: 1526

Sending regularly contexts as parameters and remember them is dangerous, read here: http://developer.android.com/resources/articles/avoiding-memory-leaks.html

Much better is to subclass Application and to make singleton with it (the application object is context too). Such sollution doesn't have significant drawbacks.

Upvotes: 1

Bozho
Bozho

Reputation: 597106

This is the Service locator pattern - you pass around a service locator (often called "Context") and get the required dependencies from it. It is not an anti-pattern, and is not that much of a bad design, but usually dependency injection is considered superior.

What you are doing is - passing the service locator even further down the object graph. What is advisable is to give each class only the dependencies it needs. So instead of passing Context in the constructor, you pass all the strings it requires. That way you won't be violating the Law of Demeter

Upvotes: 7

Related Questions