web3j
web3j copied to clipboard
Why are those decode methods in class TypeDecoder not public?
@SuppressWarnings("unchecked")
static <T extends Type> T decode(String input, int offset, Class<T> type) {
if (NumericType.class.isAssignableFrom(type)) {
return (T) decodeNumeric(input.substring(offset), (Class<NumericType>) type);
} else if (Address.class.isAssignableFrom(type)) {
return (T) decodeAddress(input.substring(offset));
} else if (Bool.class.isAssignableFrom(type)) {
return (T) decodeBool(input, offset);
} else if (Bytes.class.isAssignableFrom(type)) {
return (T) decodeBytes(input, offset, (Class<Bytes>) type);
} else if (DynamicBytes.class.isAssignableFrom(type)) {
return (T) decodeDynamicBytes(input, offset);
} else if (Utf8String.class.isAssignableFrom(type)) {
return (T) decodeUtf8String(input, offset);
} else if (Array.class.isAssignableFrom(type)) {
throw new UnsupportedOperationException(
"Array types must be wrapped in a TypeReference");
} else {
throw new UnsupportedOperationException("Type cannot be encoded: " + type.getClass());
}
}
I have to use reflection when I want to decode the input data of an transaction. https://github.com/web3j/web3j/issues/489
private <T extends Type> T decode(String input, int offset, Class<T> type) {
try {
Method refMethod = TypeDecoder.class.getDeclaredMethod("decode", String.class, int.class, Class.class);
refMethod.setAccessible(true);
return (T) refMethod.invoke(null, input, offset, type);
} catch (Exception e) {
throw new UnsupportedOperationException("Type cannot be encoded: " + type.getClass());
}
}
Or create a new class under the same package.
package org.web3j.abi;
import org.web3j.abi.datatypes.Type;
public class TypeDecoderMethodExposer {
public static <T extends Type> T decode(String input, int offset, Class<T> type){
return TypeDecoder.decode(input, offset, type);
}
}
But neither of them are elegant. What's the purpose of making those methods non-public?
Same question here...
Same question. It's pretty logical to make such utility method public
Same here, it seems the library is aggressively hiding things with the intended use case of dApps that write, but don't really read much.
I agree with the usefulness of having an accessible decode utility. I'm assuming the design decision behind keeping TypeDecoder and TypeEncoder package protected is intended, to keep them encapsulated inside DefultFunctionlEncoder and DefaultFunctionalReturnDecoder. This is likely to deal with future variations of ABI specification.
We can look into introduction ABI utils package for that purpose.
FYI, https://github.com/web3j/web3j/pull/1782 is a fix, please comment or +1 there.
@monperrus @monperrus my recommendation would still be to create a util class that extends FunctionalEncoder and DefaultFunctionalReturnDecoder to provide high-level shorthand APIs for these ABI decode/encode methods.
Even nicer would be a similar interface that handles the offsets nicely, maybe something of the shape:
public class GenericAbiHandler {
public GenericAbiHandler(String input) {/*...*/}
public <T extends Type> T decodeNext(TypeReference<T> type) {/*...*/}
public static String encode(Collection<TypeReference<?>> types, Object... data) {/*...*/}
}
Possibly spread across two classes instead, one for encoding, one for decoding.
The point being that the ABI encoding/decoding as it stands could be considered in a similar way to JSON, just a way of encoding/decoding data that should be available free of the current concrete "conveniences" for common uses.
I like your ideas @mohamedelshami @sanguivore-easyco, we need a better high-level API.
Yet, this PR alone is valuable and fully backward compatible, I would suggest to merge it first and continue later with more heavyweight engineering.
@monperrus happy to be pragmatic here and merge this for now, can you please add some documentation to what we've discussed about risk of breaking future compatibility and reference this issue in the notes?
@mohamedelshami sure! See https://github.com/web3j/web3j/pull/1782/commits/de2a890395a3ad30d85c6139c71bc2e7ddb79871
Thanks, @monperrus. Can you please make that comment more explicit?
e.g. "The public static methods can break future compatibility, and will likely replaced with high level util APIs"
For the record, methods decode* will be public as of v4.9.5.
Thanks all for your inputs.