UserAgent should expose a function to check for requirements

[yacaovsnc]

As of now, the only way to know UserAgent won't work is to call it, then expect an IllegalStateException.

It would be good to expose the requirement checking function publicly, so we can determine if UserAgent is suitable or not, and then decide if we should go ahead to make the call, or fall back to other mechanism.

As of now I am controlling code flow by exception, and I don't think that's a good pattern.

[olivierdagenais]

Ah, I think I see what you mean: UserAgentImpl, in encode() will call the package-visible determineProvider() method, which will throw IllegalStateException if none of the available providers can be used.

You can check for requirements: subclasses of Provider are required to implement checkRequirements(), which returns a List<String> representing requirements that weren't met. Here's how the GCM4ML does it, with the method being called with Provider.PROVIDERS (which, as of this writing, contains only the JavaFxProvider, but the method is future-proofed):

    /**
     * Asks all the supplied {@link Provider} implementations to check their requirements and
     * report only if all of them are missing something.
     *
     *
     * For example, suppose we have support for both JavaFX- and SWT-based browsers:
     * we just need to have one of those working on the user's computer.
     *
     * So, if they are running on Java 6, they can't use JavaFX, but that's fine,
     * because they installed xulrunner and the SWT-based browser should work.
     *
     * @param providers a list of {@link Provider} implementations to interrogate
     * @return a list of requirements, per provider,
     *          if no single user agent provider had all its requirements satisfied
     */
    static List<String> checkUserAgentProviderRequirements(final List<Provider> providers)
    {
        final List<String> results = new ArrayList<String>();
        final LinkedHashMap<Provider, List<String>> requirementsByProvider = new LinkedHashMap<Provider, List<String>>();
        int numberOfProvidersWithSatisfiedRequirements = 0;
        for (final Provider provider : providers)
        {
            final List<String> requirements = provider.checkRequirements();
            if (requirements == null || requirements.isEmpty())
            {
                numberOfProvidersWithSatisfiedRequirements++;
            }
            else
            {
                requirementsByProvider.put(provider, requirements);
            }
        }
        if (numberOfProvidersWithSatisfiedRequirements == 0)
        {
            for (final Map.Entry<Provider, List<String>> entry : requirementsByProvider.entrySet())
            {
                final Provider provider = entry.getKey();
                final List<String> requirements = entry.getValue();
                results.add("The " + provider.getClassName() + " user agent provider has the following unmet requirements:");
                for (final String requirement : requirements)
                {
                    results.add(" - " + requirement);
                }
            }
        }
        return results;
    }

It sounds like you would like a method like this to be added to Provider. The difficulty is in selecting a suitable return value. Would you be OK with the current behaviour returning an empty list if at least one Provider will work and otherwise a list of strings representing lines of a long message enumerating providers and their unmet requirements? (see the tests of the checkUserAgentProviderRequirements() method in GCM4ML's ProgramTest class for examples)

[olivierdagenais]

It sounds like you would like a method like this to be added to Provider.

...or to UserAgent. Either way, I can import the method from GCM4ML; we just have to decide what it should return such that the output can be consumed whether the target program is console-based on GUI-based.

[olivierdagenais]

OK, based on a quick conversation, I would like to propose the following methods be added to UserAgent:

    /**
     * Scans the {@link Provider} implementations, checks their requirements
     * and tries to find one that will work, preferring the one named by the
     * {@code userAgentProvider} property, if it was set.
     *
     * @return {@code true} if a suitable provider was found;
     *         {@code false} otherwise.
     */
    boolean determineProvider();

    /**
     * Scans the {@link Provider} implementations, checks their requirements
     * and tries to find one that will work, preferring the one whose
     * name was provided.
     *
     * @param  userAgentProvider the name of the preferred provider or
     *                           {@code null} if there's no preference
     * @return {@code true} if a suitable provider was found;
     *         {@code false} otherwise.
     */
    boolean determineProvider(final String userAgentProvider);

    /**
     * Determines unmet requirements (if any) for each available {@link Provider} implementation.
     *
     * @return a map {@see Provider} instances to their list of unmet requirements.
     */
    Map<Provider, List<String>> getUnmetProviderRequirements();

...what do you think? I'm not sure if determineProvider() is the best name for a method that returns a boolean; would hasCompatibleProvider() be more appropriate?

[yacaovsnc]

Looks good, and I think hasCompatibleProvider() is the more appropriate name.

[olivierdagenais]

Fixed by #21.