I was wondering if it is considered bad practice to call the outer Class' method in an inner Class and then use the inner Class' method in the outer class.
In this case: In BidParser I call the method updateMaps(), which belongs to the outer class. Additionally, I am calling in BidParser the methods of the second inner Class InputSanityChecker.
Is it bad practice and an anti - pattern? Am I creating a God object here?(more functions to follow in other external classes though)
EDIT: I am having two variables Var1, Var2(let's say) that belong to the Outer but are needed for the updateX and checkX methods.
public class Outer{
public static void main( String[] args ){
if(args.length == 1){
File file = new File(args[0]);
BidParser.parseBids(file);//<--- Question refers here
}else{
BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
BidParser.parseBids(br); //<--- and here
}
}
private static void updateMaps(String[] elements){
//.....blah blah
}
static class BidParser{
public static void parseBids(File file){
//.....blah blah
InputSanityChecker.checkInput(elems);//<---second inner class method
InputSanityChecker.checkMaps(elems); //<---second inner class method
updateMaps(elems); //<-----outer class method
}
public static void parseBids(Reader reader){
//.....blah blah
InputSanityChecker.checkInput(elems);//<---second inner class method
InputSanityChecker.checkMaps(elems); //<---second inner class method
updateMaps(elems); //<-----outer class method
}
}
static class InputSanityChecker{
public static boolean checkInput(String[] elements){
//.....blah blah
}
public static boolean checkMaps(String[] elements){
//.....blah blah
}
}
}
It is not cyclic reference. All classes - outer and nested static - are to compiler equally independent. And while you are calling static methods, there are no instance references.
This design violates Single Responsibility Principle: BidParser should be responsible for parsing bids, and that's all. I.e. this class should take input - not even File, just Reader, - and produce some Bids object, which it should return to the caller.
Then it is the caller's responsibility 1) to prepare input in the form of any Reader and 2) to take produced Bids object and to do something with it. Reader can be an instance of FileReader, BufferedReader, StringReader and so on... See Java IO Documentation
Also, this design violates Don't Repeat Yourself principle. You can see duplicate code in BidParser. This violation would be fixed automagically once you design the class to work only with more abstract input.
Considering InputChecker, if each element is checked inpedendently of the others, this class should be responsible for checking only one checkable chunk (element) at a time. And it should be the parser's responsibility to iterate over elements and call InputChecker as needed.
If there are some variables in the outer class, that are needed to parse and check bids, you should pass them as arguments. If failed check can not prevent parsing, then you'd better factor checker out of parser. So it looks like:
try{
Bids bids = BidParser.parse( bidsInput );
BidChecker.check(bids, var1);
Maps.update(bids, var2);
} catch (...){
}
To generalize: such design is bad, because it injects into the BidParser class knowledge about internals of its clients, i.e. tight coupling which should be avoided as it is not testable and leads to poor maintainability. Your class should not know anything about it's clients other than passed through arguments. And (which is overkill in this short example) the concept of inversion of control (and following dependency injection) goes even farther in pursuit of loose coupling and producing more testable and clean design.
Consider SOLID principles of Object-Oriented Design. Also Wikipedia article on Don't Repeat Yourself links to another useful principles, which introduce some kind of programming philosofy.