mbrookhart edited a comment on pull request #6152: URL: https://github.com/apache/incubator-tvm/pull/6152#issuecomment-668237667
> I dont think it break single responsibility - the code is doing conversion and a single configuration param denote which conversion it is. Another way to think about it is that that visitor is declaring a 'scoped mutator' and toanf/tobbnf is two subclass of it. > We can refactor to the latter later if needed. That is exactly breaking the SRP, it's a class that does two mutually-exclusive conversions :) The SRP-compliant way to do this would be exactly what you said, a base class and two subclasses. That being said, since we're not allowing anyone to inherit from this class, and the constructor is private, it's probably not the end of the world, I'm cool with leaving it the way it is. ---------------------------------------------------------------- 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: us...@infra.apache.org
