IGNITE-25515 Remove obsolete code from MessageReader/Writer #12110
Conversation
| @@ -683,7 +663,6 @@ else if (Map.class.isAssignableFrom(type)) { | |||
|
|
|||
| /** | |||
| * @param type Field type. | |||
| * @param name Field name. | |||
| * @param colItemType Collection item type. | |||
There was a problem hiding this comment.
I don't think we should remove @param name documentation here, as field name is still used by the method implementation.
| @@ -308,17 +305,11 @@ public void prepareDirectMarshal(CacheObjectContext ctx) throws IgniteCheckedExc | |||
|
|
|||
| } | |||
|
|
|||
| assert key != null; | |||
There was a problem hiding this comment.
Do I undestand correctly that we want to make outside code responsible for this sort of checks? From codegeneration standpoint of view this code doesn't make sense, but we may want to make this information explicit - in javadoc of Message interface or somewhere else.
There was a problem hiding this comment.
Do I undestand correctly that we want to make outside code responsible for this sort of checks?
Yes.
but we may want to make this information explicit
which information do you mean?
There was a problem hiding this comment.
which information do you mean?
My point here is that I want to describe explicitly the fact that message itself knows nothing about semantics of its fields and what are possible values for the fields. So enforsing rules like "when this field is null that field should be positive" is not a responsibility of a message itself but of a code that uses (consumes) the message.
| * @param dflt Default value if field not found. | ||
| * @return {@code int} value. | ||
| */ | ||
| public int readInt(String name, int dflt); | ||
| public int readInt(int dflt); |
There was a problem hiding this comment.
I wonder if we can get rid of this method too as its implementation simply delegates to readInt w/o parameters, effectively ignoring any value passed into as dflt.
There was a problem hiding this comment.
I'll remove this method.
| ? (T)new KeyCacheObjectImpl() | ||
| : null; | ||
| } | ||
| // /** {@inheritDoc} */ |
There was a problem hiding this comment.
Could you please leave a comment here clarifying why this peice of code is commented out and when it can be uncommented?
There was a problem hiding this comment.
I'll remove this code at all.
timoninmaxim
force-pushed
the
IGNITE-25515__msg_read_write_clean
branch
from
cead2cc to
4b92656
Compare
|
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
The Contribution Checklist
The description explains WHAT and WHY was made instead of HOW.
The following pattern must be used:
IGNITE-XXXX Change summarywhereXXXX- number of JIRA issue.(see the Maintainers list)
the
green visaattached to the JIRA ticket (see TC.Bot: Check PR)Notes
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.