-
Notifications
You must be signed in to change notification settings - Fork 407
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
Added API for easy switching between cameras #27
Conversation
@NonNull | ||
private Fotoapparat fotoapparat; | ||
|
||
private boolean started = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default is false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like to make things explicit to indicate that I intentionally wanted it to be false. My OCD :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverse ocd here :)
* Switches between different instances of {@link Fotoapparat}. Convenient when you want to allow | ||
* user to switch between different cameras or configurations. | ||
* <p> | ||
* This class is not thread safe. Consider using it from a single thread. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am afraid that users will overlook this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One should always assume that things are not thread safe. This JavaDoc just confirms it.
} | ||
|
||
private void takePicture() { | ||
PhotoResult photoResult = fotoapparat.takePicture(); | ||
PhotoResult photoResult = fotoapparatSwitcher.getCurrentFotoapparat().takePicture(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about we delegate the FA's actions like takePicture
in the FAS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically this and capabilities and autofocus
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I also thought about it, but it would explode the API of Switcher and we'll always have to support it. I suggest we proceed with current API, and then after 1.1, we'll see how big API of Fotoapparat will become. If it won't grow noticeably, then we'll do as you say.
As requested by #20