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

Question: 2 Query constructors assume attribute name as the resultAlias #38

Closed
maheshkelkar opened this issue May 27, 2015 · 4 comments
Closed

Comments

@maheshkelkar
Copy link
Contributor

Following 2 constructors for Query assume resultAlias as the attributeName.

This works fine if we are polling limited set of attributes and attribute names are self-explanatory. However, with the #34 we are allowing capability to export all the attributes and that IMO makes this assumption unnecessary.

If user does not specify the resultAlias, then we can rely on ResultNameStrategy to generate the appropriate name comprised of <objectname>.<attribute-name>.

    /**
     * @see #Query(String, String, String, Integer, String, String, ResultNameStrategy)
     */
    public Query(@Nonnull String objectName, @Nullable String attribute, @Nonnull ResultNameStrategy resultNameStrategy) {
        this(objectName, attribute, null, null, null, attribute, resultNameStrategy);
    }

    /**
     * @see #Query(String, String, String, Integer, String, String, ResultNameStrategy)
     */
    public Query(@Nonnull String objectName, @Nullable String attribute, int position, @Nonnull ResultNameStrategy resultNameStrategy) {
        this(objectName, attribute, null, position, null, attribute, resultNameStrategy);
    }

Sponsored by: Lookout, Inc.

@maheshkelkar
Copy link
Contributor Author

@cyrille-leclerc please let me know your thoughts on this.

@cyrille-leclerc
Copy link
Member

@maheshkelkar I am super busy with my day job (CloudBees).

Can you be patient? I could more easily work on this next week.

Sorry, Cyrille

@maheshkelkar
Copy link
Contributor Author

@cyrille-leclerc Sure, I understand. But, whenever you take a look, please respond to all the issues & pull requests (i.e. in bulk), so that we can make good progress.

Also, dont forget about it :)

@kerlandsson
Copy link
Member

I'm closing this due to its age and the many changes since its report.

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

3 participants