From scorp@btinternet.com Sat Oct 09 05:12:00 1999 Path: news.bctel.net!newsfeed.bctel.net!sol.pdnt.net!nntp1.savvis.net!nntp2.savvis.net!nntp1!newsfeed.stanford.edu!newsfeed.berkeley.edu!dispose.news.demon.net!demon!newsfeed.icl.net!btnet-peer!btnet!mendelevium.btinternet.com!not-for-mail From: scorp@btinternet.com (Dave Harris) Newsgroups: comp.lang.java.advocacy,comp.lang.java.programmer Subject: Re: The intractable program Date: Sat, 9 Oct 1999 13:12 +0100 (BST) Organization: Burry Holms Research Message-ID: References: Reply-To: brangdon@cix.co.uk NNTP-Posting-Host: host5-99-48-207.btinternet.com X-News-Software: Ameol2 X-URL: http://www.ameol.com Lines: 170 Xref: news.bctel.net comp.lang.java.advocacy:69112 comp.lang.java.programmer:181370 a1b84881@mail.bctel.ca (Roedy Green) wrote: > It is a nightmare of nested switches. [...] > I once tried rewriting it in pure OO. That attempt was even harder to > maintain and I abandoned it. I don't know about OO, but I don't see that the switches are helping. > I challenge someone to rewrite the code, or at least show a plausible > outline of HOW you would organise the code with pure OO in an > easy-to-maintain manner. What was wrong with the obvious approach of turning the F_console and D_Unicode etc into separate classes and using double-dispatch and/or the Visitor pattern and/or Method Object? I'm not going to write the whole thing for you, but you would get a structure vaguely like the following. (More discussion after the code.) class Visitor { // Override to provide default behaviour. String Visit( F_ fileType, D_ dataType ) { return ""; } // Override to provide behaviour specific to sequential // files but general to any datatype. String Visit( F_sequential fileType, D_ dataType ) { return Visit( (F_) fileType, dataType ); } // Override to provide behaviour specific to sequential // files and the ASCII datatype. String Visit( F_sequential fileType, D_ASCII dataType ) { return Visit( fileType, (D_) dataType ); } // Fill out the N*M combinations. // Add other convenient context and forwarding functions. private How how; public F_ dataType() { return how.dataType(); } protected String vNew() { return how.vNew; } protected void setVNew( String s ) { how.vNew = s; } } public class F_ { public abstract String accept( Visitor v ); } public class F_sequential extends F_ { public String accept( Visitor v ) { return v.dataType().accept( v, this ); } } // Fill out the M file classes. public class D_ { public abstract String accept( Visitor v, F_console fileType ); public abstract String accept( Visitor v, F_sequential fileType ); // Fill out other M cases. } public class D_ASCII extends D_ { public String accept( Visitor v, F_console fileType ) { v.visit( fileType, this ); } public String accept( Visitor v, F_sequential fileType ) { v.visit( fileType, this ); } // Fill out other M cases. } // Fill out the N data classes. class How { private String howOpenInput() { expectExceptions = true; expectExceptionOnFlush = true; expectExceptionOnClose = true; return "\n// O P E N\n"; + _fileType.accept( new HowOpenInput( this ) ); } // Other functions with their own visitors. } class HowOpenInput extends Visitor { String Visit( F_console fileType, D_ dataType ) { setVNew( "isr" ); return "InputStreamReader " + vNew() + " = new InputStreamReader(System.in);\n"; } String Visit( F_sequential fileType, D_ dataType ) { setVNew( "fis" ); return "FileInputStream " + vNew() + ";\n" + raisesFileNotFound( "FileInputStream" ); } String Visit( F_sequential fileType, D_ASCII dataType ) { setVNew( "fr" ); return "FileReader " + vNew() + ";\n" + raisesFileNotFound( "FileReader" ); } // fill out the N * M cases. // Add convenience methods. String raisesFileNotFound( String name ) { return raises( vNew() + " = new " + name + "(" + tempIn() + ");\n", "FileNotFoundException" ); } } // Fill out the remaining operations. Obviously it is a lot of code however you write it. There are potentially 10 * 6 * 19 different combinations of file type, data type and operation. I outline this approach because it results in code structured a very similar way to your switch-fest, which presumably is what you decided was best. That is, the code is grouped by operation. This is not especially OO. Eg it's not easy to add a new file type or data type. All the visitor buys me is freedom from lexical scoping, so I can split up those huge, monolithic classes and routines. Where you had howOpenInput as a 200+ line method, this has a 200+ line class made out of functions which are individually much shorter. I would argue that this is at least a little bit better. Short routines are easier to understand. One side-advantage is that the class defines a specific context, which makes it easy to add helper routines (like raisesFileNotFound()) which might not be appropriate for eg HowOpenOutput. It provides a local scope into which you can put local things, which present opportunities for more refactoring. Regardless, I'd argue that this is at least no worse. Not using switches hasn't cost me anything in maintainability. I've actually gained a little because I can use the "abstract" keyword to ensure base methods get overridden properly. > Every time I bring up a proposal for adding enums to the Java > language, somebody says smugly "They are not necessary. Only babies > use them. OO makes them totally obsolete." Even given that you want to write it with switches, I don't see how enums would have made a big difference. The code would be the same mess. That you used integer constants instead of enums does not affect the readability IMHO. True dynamic multiple dispatch would have helped more because the problem is really about dispatching the 10 * 6 * 19 combinations. Multiple inheritance or delegation or genericity would help because the problem is about exploiting the commonality between various subsets. (I'm not arguing with your comments on SCID here.) The other thing which would help is better string manipulation, because this problem is also about generating text. I would be tempted to implement a string expression evaluator, which understood named variables like "vNew", "class" and "source" and could evaluate things like: "%class% %vNew% = new %class%(%source%);\n" Enums are just irrelevant here. They don't address what the problem is about at all. Dave Harris, Nottingham, UK | "Weave a circle round him thrice, brangdon@cix.co.uk | And close your eyes with holy dread, | For he on honey dew hath fed http://www.bhresearch.co.uk/ | And drunk the milk of Paradise."