Allon Mureinik has posted comments on this change.
Change subject: core: Remove CompatException class and it's usages
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetCACertificateQuery.java
Line 18: if (FileUtil.fileExists(path)) {
Line 19: try {
Line 20:
getQueryReturnValue().setReturnValue(FileUtil.readAllText(path));
Line 21: } catch (IOException e) {
Line 22: getQueryReturnValue().setSucceeded(false);
This is redundant - you set false at the begging of the method.
However, it's probably not a bad idea to do something like this:
returnValue.setExceptionString(e.getMessage())
Line 23: return;
Line 24: }
Line 25: getQueryReturnValue().setSucceeded(true);
Line 26: }
....................................................
File
backend/manager/modules/compat/src/test/java/org/ovirt/engine/core/compat/TimeSpanTest.java
Line 62: public void testInvalidParse() {
Line 63: try {
Line 64: TimeSpan.Parse("1.02.03");
Line 65: fail("No exception was thrown");
Line 66: } catch (IllegalArgumentException e) {
perhaps it's worthwhile to move to JUnit 4 conventions, and just have
@Test(expected=IllegalArgumentException)
Although I'm not sure this patch is the place to do it
Line 67: // eat it, we are ok
Line 68: }
Line 69: }
Line 70:
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/FileUtil.java
Line 87: try {
Line 88: java.io.File file = new java.io.File(filename.toString());
Line 89: return new Date(file.lastModified());
Line 90: } catch (Exception e) {
Line 91: throw new RuntimeException(e);
I'd throw an IOException.
An even better solution - this method isn't used anywhere (only in
FileUtilTest) - just remove it.
Line 92: }
Line 93: }
Line 94:
Line 95: /**
....................................................
File
frontend/webadmin/modules/gwt-extension/src/main/java/org/ovirt/engine/ui/uioverrides/org/ovirt/engine/core/compat/EnumCompat.java
Line 11: */
Line 12: public static <T extends Enum> String[] GetNames(Class<T> clazz) {
Line 13: ArrayList<String> returnValues = new ArrayList<String>();
Line 14: if (!clazz.isEnum()) {
Line 15: throw new RuntimeException("Class is not an Enum: " +
clazz.getName());
I think illegal argument exception would be better
Line 16: }
Line 17: for (Enum e : clazz.getEnumConstants()) {
Line 18: returnValues.add(e.name());
Line 19: }
--
To view, visit http://gerrit.ovirt.org/12430
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I726e0f9e7fb0c9bff00b8d510dc3907d202e0fa2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
