keith-turner opened a new pull request, #5906: URL: https://github.com/apache/accumulo/pull/5906
This was done as an experiment to see if replacing usage of Pair with records is worthwhile. A lot of the usage of Pair in the code is because it used to be so cumbersome to create a quick data class w/ two elements (had to implement hashCode, equals, toString). Going forward, new code would probably use a record instead of Pair for this case. Was curious what modifying existing code would be like. So picked a place in the code that used Pair and tried changing it record. The main benefit I found from this change is replacing the getFirst and getSecond methods made the code more readable. When making this change I had to refer back to the type deceleration to know what getFirst meant. The drawbacks I observed was that the change was tedious to make and that it has the potential to introduce bugs. The potential to introduce bugs is probably higher when there is a pair of the same type that is used widely. Not sure if this is worthwhile to pursue through the codebase. If we did want to make these changes to existing code, its probably best to do it piece meal and focus on an individual usage of Pair in a PR to lower the chance of introducing bugs. I started of replacing one usage of Pair and found a second usage in the adjacent code that was actually really easy to replace, so did that one too. For that second case could declare a record in method as the usage of pair was only scoped to that method. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
