Better way to write code in java example

Hi,
I have the following classes :

GATracker.java
package to.go.app.analytics.ga;

import android.content.Context;
import android.content.res.Resources.NotFoundException;

import com.google.analytics.tracking.android.Fields;
import com.google.analytics.tracking.android.GAServiceManager;
import com.google.analytics.tracking.android.GoogleAnalytics;
import com.google.analytics.tracking.android.MapBuilder;
import com.google.analytics.tracking.android.Tracker;
import com.google.common.base.Strings;

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import olympus.clients.zeus.api.Response.CompleteProfile;
import to.go_internal.R;
import to.go.account.AccountService;
import to.talk.logging.Logger;
import to.talk.logging.LoggerFactory;
import to.talk.utils.event.EventHandler;

public abstract class GATracker implements ITracker
{
    public static final int DIM_USER_DOMAIN = 1;
    public static final String UID = "&uid";
    protected final Tracker _tracker;
    private Context _context;
    private AccountService _accountService;
    private Logger _logger = LoggerFactory.getTrimmer(GATracker.class, "ga");

    public GATracker(@NotNull Context context, @NotNull GAConfig config,
                     @NotNull AccountService accountService)
    {
        _context = context;
        _accountService = accountService;

        GoogleAnalytics googleAnalytics = GoogleAnalytics.getInstance(context);
        googleAnalytics.setAppOptOut(config.getAppOptOut());
        googleAnalytics.setDryRun(config.isDryRunEnabled());
        googleAnalytics.getLogger().setLogLevel(config.getLogLevel());
        _tracker = googleAnalytics.getTracker(config.getTrackerId());

        Integer dispatchPeriod = _context.getResources().getInteger(R.integer.ga_dispatchPeriod);
        GAServiceManager.getInstance().setLocalDispatchPeriod(dispatchPeriod);

        setUserDetails();
        addUserProfileListeners(accountService);
    }

    private void addUserProfileListeners(final AccountService accountService)
    {
        accountService.addProfileFetchedListener(new EventHandler<CompleteProfile>()
        {
            @Override
            public void run(CompleteProfile result)
            {
                setUserDetails();
            }
        });
    }

    private void setUserDetails()
    {
        if (_accountService.hasSignedIn()) {
            String currGuid = _tracker.get(Fields.CLIENT_ID);
            String guid = _accountService.getGuid().orNull();
            if (!Strings.isNullOrEmpty(guid) && !guid.equals(currGuid)) {
                _tracker.set(Fields.CLIENT_ID, guid);
                _tracker.set(UID, guid);
                _tracker.set(Fields.customDimension(DIM_USER_DOMAIN),
                    _accountService.getEmail().get().getDomainName());
            }
        }
    }


    protected void sendEvent(String category, String action, @Nullable String label,
                           @Nullable Long value)
    {
        _tracker.send(MapBuilder.createEvent(category, action, label, value).build());
    }



    private String getString(int resId)
    {
        String string = null;
        try {
            string = _context.getString(resId);
        } catch (NotFoundException e) {
            _logger.error("Could not find name for resource id:{}", resId);
        }
        return string;
    }
}

Open in new window


ITracker.java
package to.go.app.analytics.ga;

public interface ITracker
{
    void sendEvent(int category, int action);

    void sendEvent(int category, int action, int label);

    void sendEvent(int category, int action, int label, Long value);

    void sendScreen(int screenName);
}

Open in new window



Now i am using this GATracker inside another class like this :

 if (gaEnabled) {
            _gaReporter = new GATracker(getApplicationContext(), ConfigBuilder.getGAConfig(),
                _accountService)
            {
                public void sendEvent(int category, int action)
                {
                    sendEvent(getString(category), getString(action), null, null);
                }

                public void sendEvent(int category, int action, int label)
                {
                    sendEvent(getString(category), getString(action), getString(label), null);
                }

                public void sendEvent(int category, int action, int label, Long value)
                {
                    sendEvent(getString(category), getString(action), getString(label), value);
                }

                public void sendScreen(int screenName)
                {
                    String name = getString(screenName);
                    if (name != null) {
                        _tracker
                            .send(MapBuilder.createAppView().set(Fields.SCREEN_NAME, name).build());
                    }
                }
            };
        } else {
            _gaReporter = new GATracker(getApplicationContext(), ConfigBuilder.getGAConfig(),
                _accountService)
            {
                @Override
                public void sendEvent(int category, int action)
                {

                }

                @Override
                public void sendEvent(int category, int action, int label)
                {

                }

                @Override
                public void sendEvent(int category, int action, int label, Long value)
                {

                }

                @Override
                public void sendScreen(int screenName)
                {

                }
            };
        }

Open in new window


This seems a lot of code to write.. Is there any optimal way to achieve the same thing i am doing above or better way.. or is the above way ok ?

Thanks
Rohit BajajAsked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

mccarlIT Business Systems Analyst / Software DeveloperCommented:
I'm sure there is a better way of doing this, however, can you explain a bit more about what it is that this code is trying to do? My assumption is that, based on the value of "gaEnabled" you want to construct an object that either sends events (gaEnabled == true) or does nothing (gaEnabled == false), would that be correct?
0
Rohit BajajAuthor Commented:
yes your understanding is correct.
0
Rohit BajajAuthor Commented:
Hi Mccarl
Any ways to optimize the above code ?
0
Jim CakalicSenior Developer/ArchitectCommented:
There are two ways I can think of that you might refactor the implementation. Both ways the amount of code is technically the same but the implementation will feel more compact and cleaner.

The first way is what one might traditionall see in which gaEnabled is pushed into the GATracker class (probably another construct argument) and the ITracker method implementations live there but with each have a conditional testing gaEnabled.

I like that your instinct was to have separate classes, essentially choosing polymorphism over condiionals. You can extend this idea by refactoring the anonymous inner classes into top level classes tht extend GATracker -- these are probably horrible names but something like a ReportingGATracker with non-empty implementations of the ITracker methods  and SilentGATracker with empty implementatinos of the ITracker methods (EnabledGATracer/DisabledGATracker perhaps less preferable choices; GATrackerImpl and DefaultGATracker not on my list of favorites). Then, at the point of usage, the code is much cleaner:

    if (gaEnabled) {
        _gaReporter = new ReportingGATracker(getApplicationContext(), ConfigBuilder.getGAConfig(), _accountService);
    } else {
        _gaReporter = new SilentGATracker(getApplicationContext(), ConfigBuilder.getGAConfig(), _accountService);
    }

Open in new window


A third option, since it is really the ITracker behavior that varies, is to apply the Strategy pattern and use gaEnabled to choose between two classes implementing the interface, one with the empty behavior and one with the non-empty behavior. Then give the appropriate object to a GATracker instance to which it can delegate all of its ITracker behavior.

    
    ITracker tracker = gaEnabled ? new VerboseTracker() : new SilentTracker();
    Context context = getApplicationContext();
    Config config = ConfigBuilder.getGAConfig();
    _gaTracker = new GATracker(context, config, _accountService, tracker);

Open in new window


In the end, the amount of code is the same but the refactoring distributes it in a way that is more easily comprehensible because the introduction of well named classes reveals the intent of the code and raises the level of abstraction in that part of the code so that the details are available but below the surface until needed.

Hope that helps.

Best regards,
Jim Cakalic
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Java

From novice to tech pro — start learning today.

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.