Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconsider recursive generics #54

Open
robertvazan opened this issue Jun 6, 2018 · 3 comments
Open

Reconsider recursive generics #54

robertvazan opened this issue Jun 6, 2018 · 3 comments
Milestone

Comments

@robertvazan
Copy link

Hits typed as T extends GoogleAnalyticsRequest<T> are difficult to process. It creates situations where you have to cast U extends GoogleAnalyticsRequest<U> to T extends GoogleAnalyticsRequest<T>. If I try to use just GoogleAnalyticsRequest<?>, then the fluent setters return Object, which prevents setter chaining even when using only general parameters defined on GoogleAnalyticsRequest.

Consider dropping all classes derived from GoogleAnalyticsRequest and making GoogleAnalyticsRequest non-generic. Just put all parameter setters in this single class. It's less clean from OOP perspective, but it makes it much easier to implement generic hit processing code while keeping actual hit code as clean as it was before.

@robertvazan
Copy link
Author

In case you want to keep the recursive generics solution, please at least provide dynamic hit class where we can set hit type just like any other parameter. If other hit types can be converted to this dynamic hit type, it would allow processing of hits independently of the hit type. Here's an example:

public static class AnyHit extends GoogleAnalyticsRequest<AnyHit> {
	public AnyHit(GoogleAnalyticsRequest<?> original) {
		for (Map.Entry<GoogleAnalyticsParameter, String> entry : original.getParameters().entrySet())
			parameter(entry.getKey(), entry.getValue());
		for (Map.Entry<String, String> entry : original.customDimensions().entrySet())
			customDimensions.put(entry.getKey(), entry.getValue());
		for (Map.Entry<String, String> entry : original.custommMetrics().entrySet())
			customMetrics.put(entry.getKey(), entry.getValue());
	}
}

Any other hit class can be converted to AnyHit, modified or further copied, and then this AnyHit can be sumbitted. I use this solution in my code as a workaround for the recursive generics.

@robertvazan
Copy link
Author

If it's not clear how having AnyHit improves the situation, consider how my hit processing methods look now. Before, they looked like this:

<T extends GoogleAnalyticsRequest<T>> void process(T hit) { ... }

Now they look like this:

void process(AnyHit hit) { ... }

And there are no more casting issues.

@brsanthu brsanthu added this to the 2.1 milestone May 20, 2019
@brsanthu
Copy link
Owner

Thank you for this suggestion. I added AnyHit based on your suggestion and also method asAnyHit in base class so any hit specific instances can be converted to AnyHit. Deviation from your suggestion is that, it shared the state between old instance and new one. If you want separate instance, then use deepClone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants