[GitHub] [iceberg] rdblue commented on pull request #1958: Flink: remove static keyword from FlinkCatalog method

[GitBox <gi...@apache.org>]

rdblue commented on pull request #1958:
URL: https://github.com/apache/iceberg/pull/1958#issuecomment-748236369


   @zhangjun0x01, I think `static` is a good practice because static methods also don't have access to any instance state. That makes it easier to understand and reason about what the method is doing because it can only operate on its inputs. I think that is helpful for cases like `commitChanges` here: it is clear that the commit affects the `Table` passed in and not the catalog itself.
   
   A similar situation is whether to use `static` for inner classes. I think it is easier to see in that case that keeping state separate from inner classes is a good practice when the inner class doesn't need to access state from the containing class. To me, static methods should be preferred for the same reason.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org