-
Notifications
You must be signed in to change notification settings - Fork 0
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
S174 : more cleanup #17
Conversation
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.
A couple of suggestions
throw new IOException("Exception while deserilaizing.", e); | ||
public static Object deserialize(byte[] data) throws IOException, ClassNotFoundException { | ||
// Attempt to decompress | ||
try (ByteArrayInputStream byteIn = new ByteArrayInputStream(data); |
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.
could do something like this to avoid the error check
https://github.com/Worklytics/evalengin/blob/c3a95665c4a75a5452cc1aaf8bbed5a44aa4ad1d/worklytics-common/src/main/java/com/engetc/util/CompressionUtils.java#L41
try (ObjectOutputStream objectOut = new ObjectOutputStream(byteOut)) { | ||
objectOut.writeObject(obj); | ||
} | ||
byte[] serializedData = byteOut.toByteArray(); |
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.
just ask for the size byteOut.size()
, doing toByteArray
is copying the object into a new array, so consuming x2 memory if it happens to be big.
if (byteOut.size() > MAX) {
// compress
} else {
// return as is
return byteou.toByteArray();
}
public static void inject(String moduleClassFQN, Object injectable) { | ||
ObjectGraph objectGraph = getFromModuleClass(moduleClassFQN); | ||
objectGraph.inject(injectable); | ||
public static void inject(Class<?> componentClass, Object injectable) { |
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.
Why not
public static void inject(Class<?> componentClass, Object injectable) { | |
public static void inject(Class<?> componentClass, Injectable injectable) { |
?
So it can fails on compile time ?
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.
Injectable
is an annotation .. but yeah, will refactor to make things cleaer
Features
Change implications