asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Till Westmann (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Use a file to store the map from error codes to error messages.
Date Mon, 02 Jan 2017 07:36:08 GMT
Till Westmann has posted comments on this change.

Change subject: Use a file to store the map from error codes to error messages.
......................................................................


Patch Set 4:

(4 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1410/4/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties
File asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties:

Line 33: 3,1003 = Type incompatibility: function %1$s gets incompatible input values: %2$s
and %3$s
Should we consistently avoid the spaces around the "="?


https://asterix-gerrit.ics.uci.edu/#/c/1410/4/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/InvalidOperationException.java
File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/InvalidOperationException.java:

Line 1: /*
Please remove the double indentation of the comment.


Line 26: public class InvalidOperationException extends HyracksDataException {
It seems to me that this is a little too much as we are creating a new exception class for
an error code. If we do this consistently we get as many classes as error codes. I think that
a subclass of HyracksDataException can provide values, if it provides significant new functionality,
but for simple cases like this, it seems that a factory method could work just as well.
E.g we could add a method

    public static HyracksDataException create(int code, Serializable... params) {
        return new HyracksDataException(ErrorCode.HYRACKS, code, ErrorCode.getErrorMessage(code),
params);
    }

to HyracksDataException (or another class) and replace the code that throws this exception
with

    throw HyracksDataException.create(ErrorCode.INVALID_OPERATOR_OPERATION, op.toString(),
        LSMInsertDeleteOperatorNodePushable.class.getSimpleName());

If we would like to make this a little more compact, we could also import ErrorCode.INVALID_OPERATOR_OPERATION
statically.

Avoiding this class would also avoid having 2 names "InvalidOperationException" and "INVALID_OPERATOR_OPERATION"
for the same concept.

Does this make sense?


https://asterix-gerrit.ics.uci.edu/#/c/1410/4/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ProcessTupleException.java
File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ProcessTupleException.java:

Line 1: /*
Same comments as for InvalidOperationException apply.


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1410
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I344903a9961469b62a951a9095514bf671ff11ee
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Yingyi Bu <buyingyi@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-HasComments: Yes

Mime
View raw message