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 null instead of exception if no webcam detected #29

Closed
He-Pin opened this issue Jan 31, 2013 · 8 comments
Closed

Return null instead of exception if no webcam detected #29

He-Pin opened this issue Jan 31, 2013 · 8 comments
Assignees
Milestone

Comments

@He-Pin
Copy link

He-Pin commented Jan 31, 2013

kerr@DearPanda:~/git/ex-ice-client/EX-ICE-Client$ java -jar EX-ICE-Client_kerr.jar 
09:50:31.727 [main] INFO  c.g.sarxos.webcam.WebcamDriverUtils - Searching driver com.github.sarxos.webcam.ds.openimaj.OpenImajDriver
09:50:31.729 [main] DEBUG c.g.sarxos.webcam.WebcamDriverUtils - Driver com.github.sarxos.webcam.ds.openimaj.OpenImajDriver not found
09:50:31.729 [main] INFO  c.g.sarxos.webcam.WebcamDriverUtils - Searching driver com.github.sarxos.webcam.ds.civil.LtiCivilDriver
09:50:31.729 [main] DEBUG c.g.sarxos.webcam.WebcamDriverUtils - Driver com.github.sarxos.webcam.ds.civil.LtiCivilDriver not found
09:50:31.729 [main] INFO  c.g.sarxos.webcam.WebcamDriverUtils - Searching driver com.github.sarxos.webcam.ds.jmf.JmfDriver
09:50:31.729 [main] DEBUG c.g.sarxos.webcam.WebcamDriverUtils - Driver com.github.sarxos.webcam.ds.jmf.JmfDriver not found
09:50:31.729 [main] INFO  com.github.sarxos.webcam.Webcam - Webcam driver has not been found, default one will be used!
09:50:31.732 [webcam-discovery] DEBUG c.g.s.w.d.b.WebcamDefaultDriver - Searching devices
Not enough args for null OpenIMAJGrabber.startSession(int, int, double)
Exception in thread "main" java.lang.reflect.InvocationTargetException
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:601)
    at com.simontuffs.onejar.Boot.run(Boot.java:306)
    at com.simontuffs.onejar.Boot.main(Boot.java:159)
Caused by: com.github.sarxos.webcam.WebcamException: No webcam available in the system
    at com.github.sarxos.webcam.Webcam.getDefault(Webcam.java:411)
    at com.github.sarxos.webcam.Webcam.getDefault(Webcam.java:388)
    at us.sosia.media.webcam.WebCamUtils.getDefaultWebcam(WebCamUtils.java:23)
    at us.sosia.nat.client.P2PAgentor.main(P2PAgentor.java:180)
    ... 6 more

hello,i think that you should not raise an exception that the user can't know from the method sign,and may be an exception witch can be catched such like "NoWebCamException" or just return null when we get webcam.

thanks,the current implment cause an exception uncatched when there is no webcam connected to the PC,

@sarxos
Copy link
Owner

sarxos commented Jan 31, 2013

Agree, but I'm thinking if returning null value instead wouldn't be better. What do you think? This would be more clean IMHO.

I can change the code as you suggested, but this will cause new release to be incompatible with previous ones. E.g. such fragment will not compile and users will have to modify code:

class Test {
  private static Webcam webcam = Webcam.getDefault();
}

To:

class Test {
  private static Webcam webcam = null;
  static {
    try {
      webcam = Webcam.getDefault();
    } catch (WebcamNotFoundException e) {
      // do something
    }
  }
}

In case of final fields this will be more complicated, so either final modifier will have to be removed, or special static initializer method would have to be prepared. Let's assume the second one:

class Test {
  private static final Webcam WEBCAM = getWebcam();
  private static Webcam getWebcam() {
    try {
      return Webcam.getDefault();
    } catch (WebcamNotFoundException e) {
      return null;
    }
  }
}

Above will still result in setting null since all final fields has to be initialized. Therefore, I think the best choice is to not complicate the API and simply return null. Same as users would do in their try/catch blocks.

@ghost ghost assigned sarxos Jan 31, 2013
@He-Pin
Copy link
Author

He-Pin commented Feb 1, 2013

ok,cause the line 373 in Webcam class ,i can not know there is an exception ,i will catch it now

public static Webcam getDefault(){
    try {
        return getDefault(timeout);
    } catch (TimeoutException e) {
        throw new WebcamException(e);
    }
}

to this

public static Webcam getDefault() throw WebcamException{
    try {
        return getDefault(timeout);
    } catch (TimeoutException e) {
        throw new WebcamException(e);
    }
}

@sarxos
Copy link
Owner

sarxos commented Feb 1, 2013

This is doable, of course and I will add such throws closure, but please note that it will not affect end-users code since WebcamException is a subclass of RuntimeException which does not have to be obligatory caught. Therefore javac compiler won't notice this change at all :)

From the other side, if I change WebcamException to be subclass of general Exception, and force compiler to generate error on each uncaught such type exceptions, it will cause webcam-capture code to be dramatically changed (at least 58 errors in my test) and I would like to avoid this and rather create more specialized exceptions like WebcamNotFoundException disputed at the beginning or make API more intuitive (in this case by returning null instead).

In terms of this issue I would do the following:

  1. Return null if there are no webcams discovered,
  2. Throws TimeoutException directly instead of wrapping it in WebcamException when discovery cannot be performed.

@He-Pin
Copy link
Author

He-Pin commented Feb 1, 2013

@sarxos yes,return null is enough~I agree.I talk about this cause i forgot to catch the WebcamException

@sarxos
Copy link
Owner

sarxos commented Feb 1, 2013

Ok then. I will include this change in upcoming 0.3.8.

@He-Pin
Copy link
Author

He-Pin commented Feb 1, 2013

yeap,thank you

@sarxos sarxos closed this as completed in 1183329 Feb 3, 2013
@He-Pin
Copy link
Author

He-Pin commented Feb 3, 2013

@sarxos wow,thanks a lot.

@sarxos
Copy link
Owner

sarxos commented Feb 3, 2013

No problem :) Hope this will help to resolve your project issues :D

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

No branches or pull requests

2 participants