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

Return Flash mode off manually. Because Google #14

Merged
merged 5 commits into from
May 7, 2017
Merged

Conversation

Diolor
Copy link
Member

@Diolor Diolor commented May 2, 2017

Return Flash mode off manually when no flash instead of adding nullability in the parameters

@Diolor Diolor requested a review from dmitry-zaitsev May 2, 2017 18:21
@@ -29,7 +29,8 @@ public void putValue(Type type, Object value) {

private void ensureType(Type type, Object value) {
if (value == null) {
return;
throw new IllegalArgumentException("Provided " + type.clazz.getSimpleName()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea behind Parameters is that it essentially behaves as a map and map does allow null values to be written. Moreover, we should always expect null values to be returned from getValue anyway, so this check does not really protects us from anything.

@@ -71,7 +71,7 @@ public Capabilities fromParameters(Camera.Parameters parameters) {
List<String> supportedFlashModes = parameters.getSupportedFlashModes();
return supportedFlashModes != null
? supportedFlashModes
: Collections.<String>emptyList();
: Collections.singletonList(Camera.Parameters.FLASH_MODE_OFF);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that this is the only change which is needed to fix the issue. I suggest we revert everything else.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@Diolor
Copy link
Member Author

Diolor commented May 4, 2017

fixes #9

*
* @param parameters The parameters to validate.
*/
static void validate(Parameters parameters) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets make it not-static to make it properly object oriented.

validateParameter(parameters, FLASH);
}

private static void validateParameter(Parameters parameters, Parameters.Type focusMode) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update variable name. It is not necesserily a focus mode

@@ -129,4 +129,16 @@ public void selectPreviewSize_WithValidAspectRatio() throws Exception {
);
}

@Test(expected = IllegalArgumentException.class)
public void initialParameterMissing() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test tests validator and not provider. I suggest we make validator non-static and then just verify that it was called

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

Successfully merging this pull request may close these issues.

2 participants