[GitHub] [avro] martin-g commented on pull request #1569: AVRO-3421 Added more test coverage to ArraySchema

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

martin-g commented on pull request #1569:
URL: https://github.com/apache/avro/pull/1569#issuecomment-1052596487


   @KyleSchoonover Whoever created ArraySchema was following the "Gang of four" design patterns and more specifically Factory Method.
   The idea is to use a static method that does some logic to decide what instance to return.
   In the current case it always returns `ArraySchema` but later if needed we could change it to return a specialization of `ArraySchema`.
   As I said, usually the factory method has some logic, like the parsing here, but passes "ready to consume data" to the constructor that just does `this.foo = foo;`.
   
   This is what I think the original author did here.
   
   You can change it to plain constructor but what would be the benefit ?  Simpler API - maybe!
   But we cannot remove the factory method because this will break the old API.
   So you will have to copy its code in the new constructor. Not good.
   
   My way of thinking is: if no one complained about it then no one has problem with it. No need to fix something that is not broken.
   There are plenty of open tickets in JIRA where users have reported real problems! 


-- 
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: issues-unsubscribe@avro.apache.org

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