If you have been a programmer for more than two or three years, you have probably been significantly slowed down by someone else’s messy code. If you have been a programmer for longer than two or three years, you have probably been slowed down by your own messy code. The degree of the slow-down can be significant. Over the span of a year or two, teams that were moving very fast at the beginning of a project can find themselves moving at a snail’s pace. Every change they make to the code breaks two or three other parts of the code. No change is trivial. Every addition or modification to the system requires that the tangles, twists, and knots be “understood” so that more tangles, twists, and knots can be added. Over time the mess becomes so big and so deep and so tall, they can not clean it up. There is no way at all.

As the mess builds, the productivity of the team continues to decrease, asymptotically approaching zero. As productivity decreases, management does the only thing they can; they add more staff to the project in hopes of increasing productivity. But that new staff is not versed in the design of the system. They don’t know the difference between a change that matches the design intent, and a change that thwarts the design intent. Furthermore, they, and everyone else on the team are under horrific pressure to increase productivity. So they all make more and more messes, driving the productivity ever further towards zero.

Eventually the team rebels; and they inform management that they cannot continue to develop in this horrific code base. They demand a redesign. Management does not want to expend the resources on a whole new redesign of the project, but they cannot deny that productivity is terrible. Eventually they bend to the demands of the developers, and authorize the grand redesign in the sky.

A new tiger team is selected. Everyone wants to be on this team because it’s a green-field project. They get to start over and create something truly beautiful. But only the best and brightest are chosen for the tiger team. Everyone else must continue to maintain the current system.

Now the two teams are in a race. The tiger team must build a new system that does everything that the old system does. Not only that, they have to keep up with the changes that are continuously being made to the old system. Management will not replace the old system until the new system can do everything that the old system does, on the day of the change.

This race can go on for a very long time. I’ve seen it take 10 years. And by the time it’s done, the original members of the tiger team are long gone, and the current members are demanding that the new system be redesigned because it’s such a mess.

If you have experienced even one small part of the story I just told, then you already know that spending time keeping your code clean is not just cost effective; it’s a matter of professional survival.

I like Choy's post. It proves that not everyone uses XP/TDD...that "stinky code" gets left behind. Along those lines, it is very difficult to design what I think of as a "good" design for a complex problem in a single pass. Furthermore, I don't believe that TDD necessarily produces a "good" design, but it is hell-bent for leather on solving for the requirements and ensuring that changes don't whack us into oblivion.

If we follow "A Day in the Life of a TDD Programmer," we find a test case whereby some class A's ctor is invoked and fails because it isn't yet implemented. We then modify the code such that the test case passes. Then we'd find a new test case written for exactly one requirement, modify the code to pass the test and continue. Is this fairly close? However, rarely in my experiences are requirements evolved to the point where they say "Write a class to do xyz" or "model ABC." In fact, the XP medium of stories (and KB's cards) suggest a more abstract, less-granular approach to requirements. I hardly expect a Customer to tell me: "Implement an abstract base class that specifies the behavior needed in Prototype such that your derived classes may specialize on a common, non ctor public interface." ...okay Boss, writing test case numero uno now! Rather, requirements seem more readily conveyed:

"I want it to calculate the total production volume for the day for each product type, displayed in a status bar so everyone can see it when we walk by and keep track of it so that we can do end-of-day reports and monthly reports and so forth."

At what point does Developer decide that a class even needs to be written? Where is the test case for that part of the process? Furthermore (and an attempt to actually stay on-topic) how does Clean Code express the design "thunking" that went into the decision making not fully expressed by a set of test cases that do little more than validate that the code works and, presumedly solves some written or otherwise understood requirement?

It is my opinion that "Squeaky Clean Code" should emit its design virtues by its simplicity and obviousness from a casual glance...which is certainly a lot easier to write in a sentence than produce through code in practice. It is also my opinion that "good" OO models not only the "real world" object (when there is one), but also the "understood concepts" that are GENERALLY understood by all programmers and that these understandings should be obvious in the code...that variable naming WILL convey such messages and, though others may choose a different implementation for this or that particular part, they are able to understand the choices made by the programmer based on the clarity conveyed in the design and the code.

In reality, programmer skillsets are across the board. While Ravi brings up the concept of "Professional" programmers, there is a far more reaching entity known as the "practicing" programmer. All of us should fall into the latter category, but how many times are non-programmers (those without the basis and training of CS) are a substantive portion of that set?

The basic syntax of the commonly used languages are easy enough to learn. Even "hello, world" introduces us to the many fallacies we find in the simplicity of making the computer do what we ask of it. But, certainly, no "Professional" programmer here is going to describe the job as "simple," right? There are aspects of the job that may be simple, but if it truly was simple, "they" wouldn't need "us," would they!?

One of my favorite sayings in programming projects is this: "If you don't have the time to do it right, you certainly do not have the time to do it wrong." Of course, the mere utterance of it can be grounds for being "off-shored," where iterations are much less expensive, even if it takes 5x more of them.

Of course, we can easily see in just this thread how differences of opinions among two developers can emerge a whole conflict set of what is right and what is wrong.

Ravi, I know UB, though he may not remember me...if there was ever anyone open to the ideas of others and wanted to hear someone else's opinion on a topic related to programming, it is UB. Courage and bringing your opinion are, IMO, definitely desired. You may want to reflect somewhat on your choices of words, others here are not "afraid" of offending UB by bringing to light some real or perceived inadequacy or limitation in UB's code. We're not a bunch of "backside embracers" looking for UB's approval or consent. However, I think that just about anyone can find a lot of reasons to respect UB's contributions, both here and to the industry in general. I believe that one needs to be respectful of what UB is trying to put forth in his code example and take from it the lessons that it offers while providing meaningful and constructive critique, which is obviously what UB seeks from putting it out here. Something else to be learned from this is that only a programmer who is open, sincere and necessarily thick-skinned is able to subject their opinion of what is clean code to the forray of peers. Yes, that's right. Peers. We are a peer group. I don't believe that UB has come across in any manner that suggests that he feels that he is somehow superior to anyone else here. It is a position of respect that he conveys to us by allowing us to participate on his blog. He is inviting his peers to join him in -working- through the content presented. While I don't think that you're being overtly disrespectful, I think that you're "selling" your thoughts a bit too strongly for what the situation seems to suggest. I also think that UB recognizes some of the quality of what you've brought to the table, but the TDD "method" tells us to solve one, clearly understood problem at a time...err, or to prove that it is not solved and then make it solved and provably solved? :D

In early talks with UB about XP and "test case first," he used to say (paraphrased) "Write the test case first and then create a code file with nothing in it. Run the test. It should fail. If it doesn't, that ought to surprise you!" I'm sure that my word selection don't give the vibrancy or color of UB's public speaking skill, but it should convey the point. His position is from one of solving a specific problem as illustrated in the test case(s). That "mode" of thinking centers the mind to perform a task with as much simplicity as is needed to get the requirement implemented in a validated manner. My point here is to say that the design criteria and thought process that went into solving the problem are (often?) not (necessarily?) conveyed in a reflection of code. Even a comparison of the test cases and the resultant code may not be enough to make the design choices obvious. However, I believe that Clean Code should somehow convey that thought process such that anyone else looking at it can easily understand what the code does, what problem it solves and why the developer chose that particular path...and, not as if that isn't difficult enough, that it should do so from a casual glance, not an interrogation of every semicolon and mindless page turning of its printed or e-form documentation. It isn't difficult to figure out why I wrote "magically understandable!"

Obviously, from the perspective of Professional programmers, I am an idealist. From the perspective of a "practicing" programmer who is not necessarily a "Professional," I may as well cut my ear off because my art is entirely misunderstood. :)

However, for a moment, let's just say that what I'm suggesting is even reasonably possible by some substantial portion of all Professional programmers. At what cost in time=money does it take to bring it into existence? It doesn't "smell" like a very good candidate for automation by code generators, novices or non-practicing professionals, does it? It doesn't sound too much like a candidate for really complex algorithms that don't fit well into a divide and conquer methodology? So what does the "Pragmatic" programmer do? Design the fine art of code on every iteration or bang out the minimum, validated/able requirement one at a time until done?

There is a lot to be appreciated in both and a lot more to be appreciated when both are met at the same time. That is, in a nutshell, the definition of idealism. We want the quality of fine art at the price of scrap metal...cheaper, if possible. And I can tell you one thing for sure, a "Rob Wehrli" original goes for a lot less than a "van Gogh" original. In that respect, we all have areas where improvement can be made.

To that end, I'm not convinced that a book on Clean Code is viable. I don't doubt that UB can sell it to a publisher, rather, is the market for Clean Code *that* interested in reading it? Are the lessons in it found only after digging through it deeply enough? Does it require the MF-mentioned formatting that seems forever lost in the rest of this noise? If one can not make the argument for commenting code, how will a right-side page of commentary help? Isn't it just code comments removed from the code?

Don't get me wrong, I'm definitely not a naysayer when it comes to this topic nor do I want to be pessimistic about it. I, if anyone, am desperately searching for an answer. I strongly believe that it is an education issue. I also strongly believe that there is absolutely no requirement (other than mixed conventions) for anything representative of Clean Code in the world of software and that TDD does nothing to ensure that Clean Code is a by-product of its use. I believe that it is a Responsible Programmer issue.

I don't believe that a curriculae change at the education front is in-progress or even remotely a possibility until somehow "the forces that be" decide that they will no longer accept "ugly" code, which is an antonym of Clean code...though I'm now strongly leaning toward Choy's "stinky" as yet another improvement in my own shallow ability to find answers. Education caters to the student. Talk about the blind leading the blind! Actually, there are some very good CS schools out there in much the same way that there are some very good practitioners of programming out there. But, there are a lot of smelly ones out there, too!

To me, the ideal Clean Code book is a deodorizer. It is language agnostic. But since the concept of code requires a language, we're back into the mix with the pain of drudging through a book of mostly Java code when we're really C++ programmers or elsewise? We're back to finding flaws or even downright disagreeing with the design/implementation choices of its author...if we're even considering it because we're not already sure that our code isn't already worthy of being called a Rose by any other name.

Is the same cleanliness aspect of Clean Code present in both assembly language and Java? How about in Ruby vs Perl? Or BASIC vs Common Lisp? Are the same "talking-points" eligible for the book if we use the same litmus test as; where must my derived class be usable? Will the same message come out regardless of language?

I suspect that the answer is no...particularly as it may apply to the gems of insight brought to us by the Args example. How quickly will I return the book to its spot on the shelf (even if virtual?) if I am not satisfied with the message that doesn't pertain to a particular language, which obviates the need for "special" formatting. Maybe then, the book embraces many languages? So I'm buying 1/11th of the content that I "really want?" ...and at full purchase price?

I don't know, UB, that I have anything truly useful to say here but that it is certainly a massive undertaking in my mind. I see almost a series...Clean Code: Java, Clean Code: C, Clean Code: C++, Clean Code: SNOBOL (sorry, had to throw that in there...), and with the ¿resurgence? of it, Clean Code: Objective C.

Whereby I stand behind my (much)earlier comment that I like Choy's response.

"I'd like more emphasis on the thought process less on the details of making the changes." I think, therefore I code?


Take Care.

Rob!

i'd like more emphasis on the thought process less on the details of making the changes. cleaning kinda goes like (1) recognize dirtiness (2) evaluate impact (3) if big dirtiness, figure out how to clean (4) clean. similarly i'd expect you to say something about a code smell you see - redundancy, lack or clarity, lack of testability or solution sprawl. then discuss why the smell is a big deal. perhaps discuss options for cleaning/refactoring it. it would be a great time to introduce some of those OO principles like open-closed or SRP.

i tend to comment stinky code to remind myself later. since youre so busy, i'd assume you do something similar to make sure you remember what you didnt like about the code you left. show those notes in your evolutionary samples if possible. i think it'll elucidate the code smell identification habit for your readers.

One area that is tough for all but expert coders is knowing when the code is "good enough". when is cleaning overkill. stacking books in a neat pile on the table is one thing. but going through every book you have and re-organizing your entire bookshelf is usually overkill. it would be great if you mentioned some of the refactorings you decided not to do.

along that vein, mention the difference between clean code and over-engineered code. both require extra time.

Rob: IMO, a body of code that is magically understandable at a quick glancewithout a thorough readingis a lot cleaner...

That's good advice.

Regarding the Args code in the PDF file, I'd definitely change the font to one that didn't make every "i" (eye) look like a 1 (one).

While I generally like the concept of teaching "Clean Code" to freshmen (and everyone else who leaves it a mess), I find it hard to believe that any of them are going to carefully read through the Args example thoroughly just because you say it is an example of clean code. As someone who DAILY fixes REALLY UGLY code, my heart is bleeding for any improvement at all in the state of world affairs pertaining to this topic. I think that I'd focus on the simpler concepts of quality code formatting and simplicity of what an operation should accomplish by way of its OBVIOUS implementation and name and work my way up to an entire class design. Someone who is going to read completely (twice!) through Args already has the discipline to write Clean Code...and anyone who already has the discipline won't read it thoroughly because the tidbits of knowledge is hidden within it more than made apparent by it. IMO, a body of code that is magically understandable at a quick glancewithout a thorough readingis a lot cleaner... :D

There are primitives in Java because Java never was supposed to be "pure OO" - not even closely.

The purpose of Java was to be a language that C++ developers could easily understand, was easier to learn and use, and reasonably fast. Primitives were introduced mainly because arithmetic with objects was perceived as being too slow.

I agree that the code fragment shown is not the best way to do it. Ideally, we'd store the values in a Map<Character,Integer>. But since C does not have a Map like construct, it seems like the example was an attempt to roll his own map. Nevertheless, the idea behind not using switch statement is correct.

While I agree with the sentiment in Conrad Weisert's article (<http://www.idinews.com/caseTable.html>) I take some exception to the following example

---

 while(cin>>ch)
   switch(ch) {
      case 'a': case 'A'
        ++acnt; break;
      case 'e': case 'E'
        ++ecnt; break;
      case 'i': case 'I'
        ++icnt; break;
      case 'o': case 'O'
        ++ocnt; break;
      case 'u': case 'U'
        ++ucnt; break;
     }

The main flaw here is the use of five separately named counters. What if we decide to count "y"s, too? If a student turned in the above, I would counsel him or her to convert the sparse characters to dense integers and use an array of counters. Here's a reasonable revision:

    while(cin>>ch)
       static char vowelTbl[] = "AEIOU";
       ch = toupper(ch);
       int chx = strchr(vowelTbl,ch) - vowelTbl;
       ++vcnt[chx];
     }

---

I find the second section quite a bit more difficult to understand than the first. I can look at the first and immediatly see what it does and how to change it. The second is not nearly so transparent. To make matters worse, it produces a vector with magic indeces (e.g. The number of 'E's is kept in vcnt[1] as opposed to ecnt). So, all else being equal, I prefer the switch/case version.

Now, clearly, if we found that we were switching on the same vowels over and over again, then a more general solution should be found; probably based on polymorphism or some other form of table lookup. However, if all we need to do is count the individual vowels in a text string, then the first solution is better than the second IMHO.

Here's a link from somebody's website that points to the pitfalls of Switch/Case constructs:

http://www.idinews.com/caseTable.html

And here's the ending from that:

Actually, switch . . . case is needed less often than many inexperienced programmers believe. The key criterion for its use is that we're doing something different for each of the cases -- a different pattern of code, not just doing the same thing to different variables.

Based on what's available at his website, I feel that Conrad Weisert has a much deeper understanding of OO and software development than many of the more famous Java luminaries. And another person whose comments are worth listening to is Mike Spille (www.pyrasun.com). In my opinion, he, too, has a much deeper understanding of software development and OO than many of the thought leaders of Java/C# community.

And some more from Conrad Weisert's website: http://www.idinews.com/atrocious.html

Enjoy. And learn.

Uncle Bob feels that Property files are more difficult to maintain than strings in code.

I disagree. Once tested, they exist and can be backed up and version controlled, etc. Just like ordinary code can be.

There is a very fundamental approach to software development that maintains that we must separate the things that change from the things that remain constant.

From this viewpoint, the text of the messages is subject to change, and new messages are likely to be added if we expect arbitrary argument "types" to be passed in from the command line. I am operating under the assumption that new types will be permitted. If so, the approach of keeping the things that change (messages) separate from the thing that remains the same (a new ArgsException[?] Object to be created whenever a "type" error occurs) makes sense.

If, on the other hand, no new types of arguments are ever going to be permitted, then Uncle Bob's method will work just fine.

I do not presume to know what my users will ask for in the future. Clearly, Uncle Bob knows what is good for the users and exactly what they want. (In the term users, I include developers who will use the code.) My ignorance of my users' intent forces me to take an approach that leads to easily maintainable code that requires practically no maintenance. Uncle Bob's deeper knowledge of his users leads him to build a structure that is more difficult to change but works just fine. And woe betide the user who asks for some change!

As for development time, since I've taken this approach several times, and the code required for my approach is less, I feel that my approach will lead to faster development.

Fast development leading to more easily maintainable software vs. software that takes a bit more time to develop and will require more time to change and maintain. You choose.

Uncle Bob feels that there should not be a string constructor for complex numbers. Well, I disagree. Anyway, this is a side issue and I will not comment anymore on complex number constructors.

Uncle Bob, refuting my comment about primitives and Java, said,

Complaining about a language is not a solution to writing clean code in that language.

Complaining about a language does have its benefits. If nobody ever complained about the limitations of the earlier versions of Java, it would not have evolved at all, we would still be using API classes that are deprecated and considered examples of bad design.

As a matter of fact, my complaint was about a deeper aspect: why are there primitives at all in Java? Why can't everything be treated as an Object? The presence of primitives is the reason that leads to methods like getString, getInt, getDouble, etc., instead of having a single method called getValue that returns an Object. If we did not have primitives, then we would have no need to create all these methods. And we would have seen immediately that getValue is the only method needed! Life would have been simpler!

Uncle Bob, disagreeing with my statement that ArgsException[?] messages differ only in content, pointed out:

If you look closely at the ArgsException cases, you'll note that there is a minor difference in the behavior of each case.

The solution I provided used the varargs properties introduced in Java5 to overcome that limitation and see the common features in these Exception messages, rather than get worried about miniuscule differences. As such, the single structure provided handles all posiible cases that arise: whether a message expects 1 argument, or three or none is immaterial.

By the way, I believe many languages support this feature, including Python, Ruby, Lisp, etc. Hence, this is not same fancy extremely rare gimmick that is being used. Come to think of it, many Java5 API classes themselves use varargs.

Ravi: Is the Integer class coupled to the String class because it accepts a String argument in its constructor?

Clearly yes. What's more, the class Integer is coupled to a very specific format for representing integers in strings. That format involves 12 particular characters {+-0987654321} that must appear in a certain order with a certain grammar.

Now in the case of integer, this coupling is mostly harmless because the string representation used is so universal that there's very little likelihood that it might cause a problem. On the other hand it should be recognized that there are integer formats that are not recognized by the Integer parser. Consider Roman numerals, or parentheses used to denote negatives. Those few programmers who had to deal with those formats could make a reasonable argument that the coupling of the standard integer format was useless to them.

Ravi: Uncle Bob, if you mean that you have to cast an Object to get the "primitive" value, then that is true. But that begs the question, why are there primitives and Objects in Java, which is supposed to be a "pure" (or nearly pure) OO language? If, as an OO fan, you are designing using OO, you'd seldom use primitives except inside classes and alwways present the Object to the world. With Java5 auto-boxing, I see no reason why the interface of a class should ever expose primitives. Casting to primitives is a limitation of the Java language because generics deals only with Objects and not primitives.

Complaining about a language is not a solution to writing clean code in that language. If you must write in Java, then you'd like to avoid using casts where possible. Creating a tool that forces the use of casts is unfortunate.

Ravi: The intent is to pass a Complex number to a method. The question of parsing the String in the constructor or some other place is not relevant, especially since in this case the constructor of the Complex number is the natural place to parse it.

I disagree. I think parsing belongs in another class. The Single Responsibility Principle (SRP) tells us that there should be a separation between format and algorithm. If we put the parsing code into a constructor of class Complex, then we automatically make a particular format preferred over another. For example, the mathematical format (3+4i) becomes preferred over the engineering format (3+4j).

There are times when the parsing format and the algorithms are kept together out of sheer pragmatism. For example, there seems little reason to keep the parsing format of an integer separate from an integer. The parsing format is so universal that there is not likely to be any harm in coupling them. On the other hand, we don't want to create a tool that forces the parsing format and the algorithms to be coupled.

Ravi: When such an option comes up, I ask myself, how is the behaviour of the various sub-classes different from that of the main class, or from each other? The answer in this case is that there is no difference! Hence, creating the sub-classes is pointless. I see that as un-necessary duplication.

If you look closely at the ArgsException[?] cases, you'll note that there is a minor difference in the behavior of each case. Some use one argument of the exception, and some use two. It's not out of the real of possibility that a new one might use three, or none. So I think the use of polymorphism is a bit more appropriate than you make out. On the other hand, I did not choose to use polymorphism because the creation of a new class for each message seemed wasteful, and a switch statement seemed more economical.

Ravi: I felt that the code illustrated by Uncle Bob was not clean, so I spoke up.

Though I disagree with your conclusion, I applaud your willingness to debate. This is how we learn. If we don't share our ideas and respectfully defend them, we can't learn anything new. The trick is not to get offended and turn the debate into a name calling contest. The most valuable form of disagreement is respectful disagreement.

Ravi: Uncle Bob, I presented a solution that involves neither polymorphism nor if..then/switch statements.

That's not quite true. You created a map of constructors, which is a form of polymorphism. In essence you created a table of functions to pointers; which is precisely how polymorphism is implemented. You also used a lookup table for the messages. This is exactly how a switch statement is often implemented.

I will grant you that a lookup table is a better approach in most cases, especially if the meassages are likely to change. However, in this case, as I explained in the article, the messages were nothing more than conveniences for programmers who wanted to print something. I don't expect them to change so I felt "OK" about leaving them in the code. I think a properties file for this little utility would be overkill. It would be another component to be deployed, another element in the installation script, another file to get lost or corrupted, another directory structure to create.

Properties files are often very good things, but they aren't free. The cost has to be worth the benefit. For this little Args program, the benefit is too small, and the cost is too high.

Ravi: And I believe that is the main aim of the professional software developer: to develop applications that are easy to maintain and enhance.

I agree. However, IMHO a properties file does not make Args easier to maintain. I don't believe the messages will be changed frequently; but the properties file will still have to be deployed by everyone who uses the function. Frankly, I believe that the need to manage the properties file would discourage most programmers from using the Args class.

Gian Franco Casula: For now I quickly browsed through your article, and correct me if I'm wrong, there is no mentioning of documentation of code as a way it clean it.

True. Though I am willing to write some comments or documents about programs if I must; I consider it a failure to express myself clearly in the code. Please don't misjudge my intent. I am a consumate and avid writer. However, if at all possible code should stand on it's own without extra documentation.

All too often documents are used as a remedy for bad code. Programmers will say to themselves, "Oh, this code is bad, I should write some comments." No, they should clean the code so that it doesn't need comments.

Again, don't misjudge my intent. Some code needs documentation (such as JavaDoc[?]) because it is part of an API that many different people must use. Even then I expect that if the code is clean the number of Javadoc comments necessary will be very little.

Uncle Bob, I presented a solution that involves neither polymorphism nor if..then/switch statements. I pointed out that polymorphism was inappropriate in this case because the only difference in the sub-classes was in the text of the message, there was no change in behaviour. Hence, no sub-classes.

Given that the different messages differed only in the content, I suggested that this data be kept separate from how it was processed. The choices were:
- Use if..else or switch blocks;
- (Internalize the messages within the class) Keep the messages as static String blocks; which was illustrated with message codes and String maps generated at class loading time; or
- (Exterrnalize the messages) Put the messages with key-value format in a properties file, read the file at class loading time and provide the ability to reload it.

The first method requires changing the code and recompiling classes even when a trivial change is required. For example, if a spelling mistake needs to be corrected, the class must be changed, compiled, and possibly (quite likely) the application would need to be brought down and re-started. All this for a simple change in spelling ina single message text! It does seem that the effort required is disproportionate to the change. Furthermore, it seems ugly.

The second option is not much different from the first, we have just moved the if .. then /switch block to the static block. It does have the small advantages that the message strings are created only once, that the function to get a message is simpler, that it works at a slightly higher level of abstraction than the first. But overall, in itself, there is not much to choose between the two.

But where it leads to is interesting. Once we see that change is still difficult using the second method, we ask ourselves, what if we keep the actual message content outside the class? Can we do it? What are the benefits? Asking these questions leeads naturally to the third technique - externalizing the messages in a properties file.

The benefits of externalizing the messages are obvious. Adding a new message is simple: add a line to the properites file! Changing a message is trivial, too. The whole class takes up less code and is easier to understand. Change does not require bringing down the application and re-starting it. (Of course, a small reload method needs to be created. But the example given showed that it was a trivial and one-time change.) And that is the greatest benefit: CHANGE IS EASY!

And all this happened because we went to a slightly higher level of abstraction in the code when we moved from the first option to the second option. That is why I prefer not to use if .. then /switch blocks in my code. It leads inexorably to the next level of abstraction where change becomes trivial.

And I believe that is the main aim of the professional software developer: to develop applications that are easy to maintain and enhance.

Once things are easy to change, then the users will ask us for more things, will tax our imagination and our imagination, and we'll end up creating new and fun things. In this particular case, we could easily give selected users the ability tio change the messages. The power that is now transferred to the users makes them more satisfied with our work.

If we end up creating a situation where we encourage our users to ask for more, and that leads us to do different and mroe creative things, then we feel happy. Also, instead of having one year's expereience repeated N times, we actually have N years of expereience!

Ravi:It does not surprise me that most developers prefer the switch/case statement and the if..else chains. Indeed, it is simpler....But what about maintanance? How easy is it to maintain: to change a message or to add new messages or types? Those are the things that concern me. Any code that requires more maintenance is uglier than code with similar functionality that requires less maintenance. Therefore, I stand by my earlier statement that Uncle Bob's example is in fact one of "dirty code", not clean code.

Ravi, as you said above, if/else chains are simpler than polymorphism. This means that, all else being equal, they are also easier to maintain. What makes an if/else chain harder to maintain than polymorphism is duplication. A duplicated if/else chain is very difficult to maintain because the same changes have to be made in each duplication. Polymorphism is easier to maintain since the same kind of change is made all in one place (i.e. the derived class).

This is actually an old dilemma. Polymophism makes it easy to add new data types to existing algorithms. If/else chains make it easier to add new algorithms to exsiting data types. Consider, for example, the problem of adding a new method to an abstract class. You have to add it to all the derivatives, just like you have to add a new case to every if/else chain or switch statement when you add a new data type. So the benefit of polymorphism is not absolute. It is relative to whether you are more likely to add more functions, or more data types. If data types are more variable than functions, then polymorphism helps. If functions are more variable than data types, then polymorphism hurts.

OO is not about the elimination of switch statements. That's just an old wives' tale that's been spread around a bit too much. OO is about managing dependencies in context. Sometimes the best way to create a maintainable module with minimal coupling is to use a switch statement as opposed to polymorphic dispatch. Again, my rule about this is that switch statements are OK so long as they aren't duplicated.

For now I quickly browsed through your article, and
correct me if I'm wrong, there is no mentioning of
documentation of code as a way it clean it.
Personally I noticed that (re)formulating the purpose
of a class, or what a method does in documentation
comments, often elicits some refactoring that
makes the code cleaner and easier to explain.

funny, a few years ago i was dissatisfied with the state of Java command line option parsers. all seemed to have fairly gnarly code, inwards and outwards. this kind of parsing didn't seem like such a gnarly problem, so i set out to build my own TDD-style. the result is JOpt Simple (http://jopt-simple.sf.net).

i think command line option parsing makes a great TDD exercise for people learning to work TDD/BDD.

It does not surprise me that most developers prefer the switch/case statement and the if..else chains. Indeed, it is simpler. Other alternatives require more thought. And the vast majority of developers prefer not to spend too much time thinking about their code.

But what about maintanance? How easy is it to maintain: to change a message or to add new messages or types? Those are the things that concern me. Any code that requires more maintenance is uglier than code with similar functionality that requires less maintenance. Therefore, I stand by my earlier statement that Uncle Bob's example is in fact one of "dirty code", not clean code.

I once saw an if..then chain with about 75 cases! Total code was about 650 lines long. I pointed out that using a function and a map, it could be reduced to about 10 lines of code. Non-programmers immediately understood the possible benefits, while programmers were not convinced! Would Uncle Bob consider the 650 lines as clean code? I don't see the difference between an if-else with 75 branches and one with 4 or more. I bet those 75 branches started out as 4 or 5 and "evolved" with time!

I think a good programmer has the ability to see patterns and remove duplication. Uncle Bob's inability to see the patterns and to remove duplication should be amazing for the vast majority of developers. His clients should realize that when his team develops something, it will always require extensive maintenance but Uncle Bob will pat himself on the back and claim his code is clean!

I don't know if you (Uncle Bob, Ravi and other guys who are reading here) can read chinese comments in my post; In fact, most of them said:"the original code is more cleaner than new solution". They love Switch/Case and think new solution is more complex. Some of they are NOT try to understand my code even. :(

Anyway, I insist my opinion.

Isn't one of the tenets of XP about courage? The courage to speak the truth and face facts.

I felt that the code illustrated by Uncle Bob was not clean, so I spoke up. The fact that many software developers wouldn't dare to question Uncle Bob did not cause me to hold back my comments. I showed an alternative to using the ArgumentMarshaler[?] classes because I thought they were ugly.

In a reply to linkcd, Uncle Bob insists that the use of switch statements keeps the code cleaner! And that they are pleasant to read. While it may be pleasant to read, it does not keep the code cleaner. It makes the code ugly. Especially when there are other alternatives.

Even if the code is pleasant to read, I'll ask a more fundamental question: Isn't it the objective of the professional software developer to write code that is easy to maintain and change? If something is pleasant to read, but requires you to modify code and re-compile classes even for trivial changes, shouldn't one look for alternatives? Failure to do so is unprofessional. Insisting that what one presented once is "clean" shows a lack of courage, a lack of willingness to admit our mistakes, and a refusal to learn. Isn't it alsoa violation of XP's principle of being courageous?

As an aside, reading Uncle Bob's response to linkcd, I see that the alternative he explores is of creating more sub-classes of ArgsException[?], one per message. I've seen this option implemented very often, especially in open-source frameworks.

When such an option comes up, I ask myself, how is the behaviour of the various sub-classes different from that of the main class, or from each other? The answer in this case is that there is no difference! Hence, creating the sub-classes is pointless. I see that as un-necessary duplication.

All that differs is the content of the exception message. That is data. I repeat, that is data, not behaviour. Hence, good OO principles would dictate that one not create a sub-class just because the data has changed.

Most OO developers need to learn to distinguish between data and code. Then we'll see less code being written, yet being able to do more and need less maintenance. While this seems paradoxical, it is not so. A little introspection reveals that because you write less code, you make fewer implicit assumptions. Assumptions limit the things your code can do. Because you assume fewer things, you prevent fewer things and permit more things to happen. Hence, code can evolve faster and be more flexible.

Uncle Bob says,

I will grant you that you did provide an option for multiple arguments. For example, with your system I could have put the following line in the Argument class:
put("c", Complex.class, String.class, String.class);and then I could invoke with:
Arsg a = new Args("c:myComplex", new String[] {"[3,4]"});This will call Complex(String r, String i) with "3" and "4". However, this is clearly not what I was after. I want to parse the string "(3+4i)"

Actually, this is not what you want. You want to pass a Complex number to a method.

The intent is to pass a Complex number to a method. The question of parsing the String in the constructor or some other place is not relevant, especially since in this case the constructor of the Complex number is the natural place to parse it.

As a developer I want the option of writing either:

 Complex c = new Complex("3+4i");

or

 Complex c = new Complex(3,4);

There is a simple way of getting rid of the switch statement in the ArgsException[?] class that I showed. To make it even more maintenance free, I use the following technique.

I have a ConfigData[?] class that reads all properties/text files when the application starts up. The main method (or its equivalent) has a line of code like this:

 ConfigData cfg = new ConfigData("config.txt"); 

Now, I would have a text file with the follwoing data about the ArgsException[?] messages:

args.exception.insufficient.values:Expected {0} arguments, found only {1}
args.exception.insufficient.types:For {1} types, found only {0} values.
...

Now, my ArgsException[?] class would look like this:

import java.text.MessageFormat;
import java.util.*;

import ConfigData;

public class ArgsException extends RuntimeException
{
	private static Map<String, String> messages_ =
		ConfigData.getConfigData().getAllValues("args.exception");

	public ArgsException(String messageKey, Object... arguments)
	{
		super(MessageFormat.format(messages_.get(messageKey), arguments));
	}
	public static reload() {
		messages_ = ConfigData.getConfigData().getAllValues("args.exception");
	}
}

So, when the class is loaded, the data would be loaded. When any change is needed, the file is changed, and the reload method is invoked. All this while the application is running.

Also, please note that there is no need to change the code in the class.

So you can get rid of the switch statement, and you can make the class very versatile and close to zero maintenance at the same time.

Uncle Bob, if you mean that you have to cast an Object to get the "primitive" value, then that is true.

But that begs the question, why are there primitives and Objects in Java, which is supposed to be a "pure" (or nearly pure) OO language? If, as an OO fan, you are designing using OO, you'd seldom use primitives except inside classes and alwways present the Object to the world. With Java5 auto-boxing, I see no reason why the interface of a class should ever expose primitives.

Casting to primitives is a limitation of the Java language because generics deals only with Objects and not primitives.

My main argument,w hich I have repeated several times, is that:
1. You do not need to have the if .. then else clause.
2. You do not need the switch statement.
3. You do not need ArgumentMarshaler[?] or its sub-classes with static get methods.

As to the argument that client code might need to cast the Object, you do not have to.
The following code works:

 Args args = new Args("s:name,i:max,d:average", "GreatOne", "100", "75.5d");
     Integer max = args.getValue("max");
     String name = args.getValue("name");
     Double average = args.getValue("average");

As you can see there is no casting involved!

Personally, I can easily see why a Complex class would have a String argument constructor and naturally. Many languages permit you to type in 3+4i (or 3+4j) and it will be interpreted as a complex number. Therefore, a string argument seems natural.

When you want to create objects from command line arguments, the only way is to assume that there is a string constructor available for the class. Integer, String, Double, Float, etc., already have that constructor. User defined classes that directly accept commands from the command line must have a string argument constructor. Hence, I do not think that this constitutes coupling. Is the Integer class coupled to the String class because it accepts a String argument in its constructor? Internally, it does parse the string.

Is it a case of coupling the parsing of the string with the class? Not so, because such classes naturally would have a string argument constructor.

Ravi: Uncle Bob, I thought you wanted some way to handle arguments passed in to main function of a Java class invoked from the command line. You should have given some examples of the command line arguments that you wanted your program to solve.

In fact, the test cases were presented in the article.

Ravi: Anyway, it is easy to modify my program to handle the arguments the way you want it.

I'm sure it is. However, you were using the "line count" argument in your previous post, so I wanted to be clear that you were solving a different problem than the one stated.

Ravi:Please do not make false statements without understanding my code.

I apologize if I was mistaken; but I'm still having trouble understanding how the parsing of the strings is decoupled from the class. Consider, again, the complex number example. I want to be able to pass a complex number in as the string: "(3+4i)". It seems to me that your solution must parse this string in the constructor of class Complex.

To be clear. If I want to handle complex arguments, I need to add this line to the static initializer of the Argument class:

put("c", Complex.class, String.class);

Then I can invoke it with:

Args a = new Args("c:myComplex",new String[] {"(3+4i)"});

This will cause the string "(3+4i)" to be passed into the constructor of class Complex. Right?

I will grant you that you did provide an option for multiple arguments. For example, with your system I could have put the following line in the Argument class:

put("c", Complex.class, String.class, String.class);

and then I could invoke with:

Arsg a = new Args("c:myComplex", new String[] {"[3,4]"});

This will call Complex(String r, String i) with "3" and "4". However, this is clearly not what I was after. I want to parse the string "(3+4i)"

jakeb said: any comments on my suggestions? :)

Actually I had quite a bit of fun cutting and pasting your code into my solution and seeing all the tests pass. It's an elegant solution to the duplication of the static get functions. Thanks for showing it to me. I still think "C++" when I see templates, and forget that the Java typesystem allows you to put an ArgumentMarshaler[?]<Integer> into a raw ArgumentMarshaler[?] reference.

My only real quibble with it is the sheer ugliness of the declaration:

public <S, T extends ArgumentMarshaler<S>> S process(Class<T> marshalerClass, char arg) {

That's a real mind-bender. There is a real temptation to simply blur your eyes and not read that process function.

If I expected that there were going to be quite a few more argument types, then I would probably adopt the use of generics as you have done. The process method definitely makes the individual marshallers easier, and regularlizes the whole problem of the get. However, if there weren't going to be many more argument types; I'd probably leave the generic code out and live with the static get methods instead. They're just so much easier on the eyes.

linkcd said: But Uncle Bob, EVERYTIME if we wanna add a new argument type, we have to modify the if-else statement in Args class, and switch-case statement in exception class. We are hit by SAME bullet again and again. I think it violates "Open Close Principle". Yes, i agree the Switch statement are pleasant to read, but if our users ALWAYS want us to support new argument type, we should better avoid if-else/switch statements.

You make a good point. The two switch statements, while not identical, are related; and this violates my rule. On the other hand there is no benefit to turning the switch statement in ArgsException[?] into a table, since it would still need to be modified under the same conditions. I could eliminate the ArgsException[?] switch statement by deferring the messages back to the ArgumentMarshaller[?], but that couples Messages (UI) with business rules and therefore violates SRP.

I think yours is a good argument for eliminating the messages in the ArgsException[?] class altogether. On the other hand, that just pushes the switch statement out into the user code.

Another solution would be to create a hierarchy of ArgsException[?] derivatives. That would get rid of the switch, but does not completely solve your OCP complaint, since we'd have to add new exception derivatives every time we added a new type.

That leaves us with eliminating the messages and pushing that switch out to the application code. I'm not happy that. So I think the switch in ArgsException[?], though it violates OCP, helps to keep user code cleaner, and maintains a good separation of concerns (SRP).

Uncle Bob said:"Switches and if/else chains may be badly abused, and may never be necessary, but sometimes they are pleasant to read. My rule for them is that they should never appear more than once."

But Uncle Bob, EVERYTIME if we wanna add a new argument type, we have to modify the if-else statement in Args class, and switch-case statement in exception class. We are hit by SAME bullet again and again. I think it violates "Open Close Principle".

Yes, i agree the Switch statement are pleasant to read, but if our users ALWAYS want us to support new argument type, we should better avoid if-else/switch statements.

To summarize my rebuttal to Uncle Bob's comments:

1. The accusation of coupling between the constructor and string parsing is not based on facts.

2. The Unix type command line can still be generated without using if .. then or switch statements, or the ArgumentMarshaler[?] classes.

The logic to do so is simple. Just read everything after the name of the command as a single string, then split it using "-" as the separator, then handle each part in a simple way. No need for the if .. then statements or switch statements or the ArgumentMarshaler[?] classes.

Uncle Bob, while your code uses constructs like "p#", "d##", etc., to identify data types, mine uses "s:name,i:maxValue". Not much of a difference, I think.

Uncle Bob, I thought you wanted some way to handle arguments passed in to main function of a Java class invoked from the command line. You should have given some examples of the command line arguments that you wanted your program to solve. The article does not make it clear that you are dealing with Unix type command line arguments only.

Anyway, it is easy to modify my program to handle the arguments the way you want it. Your if .. then clause also has implicit boolean values, and always assumes that any boolean value is true. That is a serious limitation. My suggestion overcomes that limitation.

The code I suggested can be made even more versatile so that adding different types of arguments would not require changes to any code, as I'd mentioned earlier. In that way, the solution I suggested would be more developer friendly and require practically no maintenance.

The main point I was making is that the if .. then structure and the switch statement can be eliminated; and the ArgumentMarshaler[?] classes are not needed.

Uncle Bob said, "Another problem is that since you are using the constructor of the object to parse the argument string, [I say: Uncle Bob, that is a totally false statement]you have coupled the parsing logic and the object together. For example, if you wanted to represent a complex number as "(3+4i)" you'd have to put the parsing code in the constructor of the Complex class."

That is totally false. Read the code and my comments that preceded the code. I clearly state that the only condition is that the class should have constructirs that accept one or more string arguments. Also, see the static block of the Argument class, where I give the example

put("m", Misc.class, String.class, String.class, String.class);

where Misc class has a constructor that takes three string arguments.

The Complex class would not parse the string argument as 3+4i. It could have a constructor that takes two arguments. The parsing logic is not coupled to the class.

Please do not make false statements without understanding my code.

Uncle Bob says, "This conjures up images of commands that look like this:

myCommand true 0 3.9 shazam false false true MyFile hisFile true -23

"

Interestingly, if you look at the startup files of many applications, you do see arguments of this type. But even more interestingly, this is not how my code would be invoked, as I showed by the sample invocation in an earlier post:

java ArgsTest "s:name,i:maxValue" Test 15

any comments on my suggestions? :)

No. You can write clean code in Java. You can write clean code in Python. You can write clean code in Ruby, C++, Smalltalk, and even COBOL. Writing clean code is a skill. That skill can be practiced in any lanaguage. If you don't have the skill, you will write dirty code in any language.

Ravi,

The idea of using a map of constructors is very clever. So is the templatized 'get' method. However, to use these clever tricks you had to change the original requirements. Unfortunately by doing so you crippled quite a bit of the usability of the tool.

What you have created is a positional argument system, whereas mine was a keyword argument system. In mine, the arguments may appear in any order, or even be omitted. The order in the schema is irrelevant. In yours, the order in the schema is everything. The arguments must appear in that order, and they cannot be omitted.

Suppose we have a command c that has two boolean arguments: v and d. If v is true then we get verbose output. if d is true then we get debug output. In my version I can say Args("v,d",...) and then issue the command with the -v and -d arguments in any order. Any of the following would be valid:

c
c -v
c -d
c -d -v
c -v -d
c -vd
c -dv

Whereas with your system you'd have to say this Args("b:v,b:d",...) And issue the commands like this:

c true true
c true false
c false true
c false false

Spelling out the words "true" and "false" seems unfortunate; especially when the words are not labled with their meaning. This conjures up images of commands that look like this:

myCommand true 0 3.9 shazam false false true MyFile hisFile true -23

. One of the things I really want an argument processor to do is to allow the arguments in any order, and to allow them to be omitted.

Another problem is that since you are using the constructor of the object to parse the argument string, you have coupled the parsing logic and the object together. For example, if you wanted to represent a complex number as "(3+4i)" you'd have to put the parsing code in the constructor of the Complex class. This is not a good place for it. Business rules (like Complex) should be isolated from their UIs (like the parsing format) (See: The Single Responsibility Principle).

Still another problem is that given

Args a = new Args("d:b", ...)

you can't use a simple 'if' statement like this:

if (a.getValue("f"))

instead you have to say

if ((Boolean)a.getValue("f"))

. Likewise, if the argument is a double, you can't say

double d = a.getValue("f");

. You have to use a cast, or the Double wrapper instead. That's the reason that the templatized 'get' method doesn't work as nicely as the simple static getDouble, getInt, methods that you were complaining about. By using the clever templatized method you have forced the programmers to use casts, or wrappers.

As for the argument about if/else and switch, that's a religious argument IMHO. Switches and if/else chains may be badly abused, and may never be necessary, but sometimes they are pleasant to read. My rule for them is that they should never appear more than once.

The example of "Clean Code" presented in the original article has several places where it can be improved:

Use of if .. then ... else if blocks.
Use of switch statement: Rarely ever is it needed.
Use of classes that are not germane to the problem (ArgumentMarshaler[?] and its sub-classes.)
Improper use of the method getValue within the sub-classes of ArgumentMarshaler[?]. OO philosophy would suggest that these methods not be static and not accept arguments. So would common sense.

It is interesting that with all these shortcomings one of the leading lights of the Java/C++/C# community, Uncle Bob, believes that his code is clean and should be emulated.

It has been my contention that the Java language encourages many bad practices. Over time, practitioners become so used to it that they begin to think that these bad practices are actually "good"; such as thinking in terms of "Helper" classes, not realizing that these are not pure OO, but simply procedural/functional code akin to Python modules and Ruby mixins. It just shows that Bjarne Stroustoup knew what he was talking about when he said (paraphrased), " C++ does not force you to follow any one paradigm."

Which is why there is a wealth of wisdom in what Paul Graham says (http://www.paulgraham.com/noop.html) says, "My own feeling is that object-oriented programming is a useful technique in some cases, but it isn't something that has to pervade every program you write. You should be able to define new types, but you shouldn't have to express every program as the definition of new types."

I suggest that Java practitioners pick up a simple language like Ruby or Python and read through the documentation, and really understand their philosophy. Writing small programs in these languages would help. They shouldn't get fixated on the static/dynamic typing aspects but focus on what those languages let you do, the expressiveness they give you that keeps you close to your problem domain. As opposed to Java which forces you to focus on the plumbing rather than the problem. (ArgumentMarshaler[?] classes are a good example of the type of code that such misplaced focus can lead to. And an example of the convoluted code that results from lack of first class functions in Java.)

One of the things I try hard to do is to avoid "magic" or hard-coded values in my code. If you look at my code, such values occur only when executing the static block or when the ArgsException[?] is being raised. In all the methods, there is rarely a hard-coded value.

When I see code like:

if (elementTail.length() == 0)
marshalers.put(elementId, new BooleanArgumentMarshaler());
else if (elementTail.equals("*"))
marshalers.put(elementId, new StringArgumentMarshaler());
else if (elementTail.equals("#"))
marshalers.put(elementId, new IntegerArgumentMarshaler());
else if (elementTail.equals("##"))
marshalers.put(elementId, new DoubleArgumentMarshaler());
else if (elementTail.equals("[*]"))
marshalers.put(elementId, new StringArrayArgumentMarshaler());
else
throw new ArgsException(INVALID_ARGUMENT_FORMAT, elementId, elementTail);

I feel that there could be a better way.

Usually, code like that can be replaced by Maps. I strive hard to get rid of these if then clauses, so much so that it is now second nature to me and I rarely write them.

If Java had first class functions, it would be easy to associate named functions with the "value" part of the Map. Since Java does not have them, one has to find alternatives such as the use of "Constructor" variables that can be seen in the code sample provided.

To be honest im not familiar with java, but im trying to avoid the Switch-Case code, and here is my article: http://linkcd.cnblogs.com/articles/334559.html

Written by .Net 2.0, but maybe similar to Ravi Venkataraman's Argument class concept:
static { //This could be loaded using configuration files, too.
put("b", Boolean.class, String.class);
put("s", String.class, String.class);
put("d", Double.class, String.class);
put("f", Float.class, String.class);
put("i", Integer.class, String.class);
put("m", Misc.class, String.class, String.class, String.class);
}

In the sub-classes of the ArgumentMarshaler[?] class there are methods of the type:

public class StringArgumentMarshaler implements ArgumentMarshaler {
...
public static String getValue(ArgumentMarshaler am) {
   if (am != null && am instanceof StringArgumentMarshaler)
      return ((StringArgumentMarshaler) am).stringValue;
   else
      return "";
}
}

Why should the getValue method of any ArgumentMarshaler[?] sub-class have to be passed in an ArgumentMarshaler[?] parameter? That doesn't sound like good OO to me. Ideally, it should just say

public class StringArgumentMarshaler implements ArgumentMarshaler {
...
public static String getValue() {
      return this.stringValue;
}
}

Is there any reason why this will not work?

Note that in the solution provided there is no need for the ArgumentMarshaler[?] and its sub-classes. Their functionality has been subsumed within the Argument class itself by the simple technique of using Constructors.

As can be seen, this code is less than the original code. At the same time, it has all the features of the original one. And it can be easily extended without any problem. Actually, all the code in the static blocks can be generated using properties type text files. Thus, adding a new "type" of argument is easy to do: simply add a new line of data in the text file and invoke the block of code. This could happen while users are logged on. There would be no need to re-start the application.

There would be no need to change any class with the property-file approach even when new "types" are added or new messages are added!

This reverse chronological display of comments is unexpected and hinders reading the code. We're more used to reading top to bottom.

Anyway, here's how you would invoke an ArgsTest[?] class:

java ArgsTest "s:name,i:maxValue" Test 15

Here, s:name says that there is a "s"tring variable named "name"
and "i:maxValue" says that there is an Integer variable named "maxValue".

The next two arguments are the values of name and maxValue respectively.

In the code to obtain their values, one simply says something like:

    // Using the above example, the "key" is "s:name,i:maxValue
    // and the values are: args = String{"Test", "15"}
public static void exec(String key, String[] args)
  Args arguments = new Args(key, args);
  String s1 = args.get("name");
  Integer i = args.get("maxValue");
}

Here's the code for the ArgsException[?] class.

import java.text.MessageFormat;
import java.util.*;

public class ArgsException extends RuntimeException
{
	private static Map<String, String> messages_ =
		new HashMap<String, String> ();

	static { // This could be (should be) based on a properties type file.
		messages_.put("insufficient.values",
			"Expected {0} arguments, found only {1}" 	);
		messages_.put("insufficient.types",
			"For {1} types, found only {0} values." 	);
		messages_.put("repeated.argument.name",
			"The argument named \"{0}\" appears at least twice in  \n '{1}'");
		messages_.put("invalid.argument.name",
			"There is no argument named {0}. The valid ones are {1}");
		messages_.put("invalid.type",
			"There is no predefined type with symbol \"{0}\". The valid ones are {1}");
		messages_.put("invalid.constructor",
			"Could not create a new Object of type {0} using   \"{1}\"   as the constructor argument.");
	}

	public ArgsException(String messageKey, Object... arguments)
	{
		super(MessageFormat.format(messages_.get(messageKey), arguments));
	}
}

Here's the code for the Args class:

import java.util.*;
import java.lang.reflect.Constructor;

public class Args
{
	private Map<String, Argument> args_ = new HashMap<String, Argument> ();
	private Set<String> argNames_ = new HashSet<String> ();

	public Args(String argsKey, String[] values)
	{
		int maxArguments = values.length;
		int counter = 0;

		String[] argsKeys = argsKey.split(",");
		for (String argInfo: argsKeys)
		{
			if (counter > maxArguments-1)
				throw new ArgsException("insufficient.values", argsKeys.length, maxArguments);

			String[] nameTypePair = argInfo.split(":");
			String name = nameTypePair[1];
			String type = nameTypePair[0];

			if (argNames_.contains(name))
				throw new ArgsException("repeated.argument.name", name, argsKey);
			else	argNames_.add(name);

			args_.put(name, new Argument(name, type, values[counter]));
			counter++;
		}

		if (maxArguments > counter)
			throw new ArgsException("insufficient.types", counter, maxArguments);
	}


	public <T> T getValue(String argName)
	{
		if (args_.containsKey(argName))
			return (T) args_.get(argName).value();
		else
			throw new ArgsException("invalid.argument.name", argName, this.argNames_);
	}


	public Set<String> allArgumentNames() { return this.argNames_;	}

}

The next response has the code. Note that there are only three classes that do everything that the code suggested by Uncle Bob does. These three classes are a total of 125 lines including blank lines and lines that contain only curly braces.

Note further that this code is easily extensible and there is a very clear separation of concerns. Adding a new "type" of argument only requires adding a new statement to the static block of Args class. No other change is required. Also, adding new messages requires addition of a new line of code to the static block of code in ArgsException[?] class.

Can we eliminate even this change, structuring it so that making the above changes does not result in any change to any class whatsoever? Yes, we can! If anybody is interested, I'll explain later. The code follows in the next comment.

OK. Let's see what we are trying to do here.

We'd like a nice, simple way to parse command line arguments that are given while running Java programs.

1. . There should be a very simple way for specifying each argument's name, type and value. Each argument should have all three values: name, type and value.

1. . The types specified using command line arguments must have at least one constructor that takes one or more strings as argument. This restriction is put because only Strings can be passed from the command line. An integer is passed as a string and "parsed" to an int in the main method, usually. Hence, this is not an unusual restriction. That this is easy to extend is shown by adding a small class Misc that has a constructor that takes three String arguments. For a first reading, you can ignore this Misc class.

Since the "type" information is used only to create a value of the specified type for the Argument, we do not explicilty store the type, but store the constructor with the specified number of String arguments. [It is easy to extend this to handle the case where there are several possible constructors with different number of Strings as parameters to the constructor.]

The Constructors are generated once and stored in a Map; hence performance should not be a problem.

1. . The specified type and value should be compatible, That is, if a number is expected, and an alphabet is passed, we should get a meaningful error message.

1. . It should be easy to get an argument's value simply by asking for it by the given name. Thus, if we have specified that "s" is a String variable that has been assigned the value "I love programming!", then, a simple command like

String s = args.getValue("s") should return the value "I love programming!"
Similarly, the following code should work, assuming that we have specified the type and name correctly:
Integer i = args.getValue("maxAllowed");

1. . Asking for a named value that doesn't exist should throw a Checked Exception so that the caller can handle this in whatever way they want.

The first point above suggests a class named Argument that has name, type and value; and that ensures that the specifed type and value are compatible. In the original code, the String array is essentially an array of pairs, with the name and value of each argument being passed in through the main method. In this approach, the specification of the "type" of argument is provided by a similar mechanism to that used in the original article, but with a slight difference: each element of the comma separated list in itself is a pair of values, one for the type of the argument, and the second for the name of the argument, with a colon (":") used as the separator.



There should be a class Args that essentially takes the command line arguments, and creates an array of Arguments from them. There is a simple method (parametrized over type <T> using Generics) called getValue that returns value of the appropriate type. This takes care of point 3.

There is an ArgsException[?] class that handles the exceptions thrown in a very simple way.

Further comments and code follow.

Just noticed a simplification that can be made to the Args class from what I've posted earlier:

	public boolean getBoolean(char arg) {
		return process(BooleanArgumentMarshaler.class, arg);
	}

	public String getString(char arg) {
		return process(StringArgumentMarshaler.class, arg);
	}

	public int getInt(char arg) {
		return process(IntegerArgumentMarshaler.class, arg);
	}

	public double getDouble(char arg) {
		return process(DoubleArgumentMarshaler.class, arg);
	}

	public <S, T extends ArgumentMarshaler<S>> S process(Class<T> marshalerClass, char arg) {
		ArgumentMarshaler m = marshalers.get(arg);

		if (m != null) {
			try {
				marshalerClass.cast(m);
				return marshalerClass.cast(m).getValue();
			}
			catch (ClassCastException e) { }
		}

		try {
			return marshalerClass.newInstance().getDefaultValue();
		}
		catch (InstantiationException e1) {}
		catch (IllegalAccessException e1) {}
		return null;
	}

Then your marshaler classes are very simple:

public class BooleanArgumentMarshaler extends AbstractArgumentMarshaler<Boolean> {
	public void set(Iterator<String> currentArgument) throws ArgsException {
		value = true;
	}

	public Boolean getDefaultValue() {
		return false;
	}
}

public class IntegerArgumentMarshaler extends AbstractArgumentMarshaler<Integer> {
	public void set(Iterator<String> currentArgument) throws ArgsException {
		String parameter = null;
		try {
			parameter = currentArgument.next();
			value = Integer.parseInt(parameter);
		} catch (NoSuchElementException e) {
			throw new ArgsException(MISSING_INTEGER);
		} catch (NumberFormatException e) {
			throw new ArgsException(INVALID_INTEGER, parameter);
		}
	}

	public Integer getDefaultValue() { return 0; }
}

How about this:
making the ArgumentMarshaler[?] interface

public interface ArgumentMarshaler<T> {
	void set(Iterator<String> currentArgument) throws ArgsException;
	public T getValue();
	public T getDefaultValue();
}

then creating an abstract class:

public abstract class AbstractArgumentMarshaler<T> implements ArgumentMarshaler<T> {
	protected T value;
	public AbstractArgumentMarshaler() { value = getDefaultValue(); }
	public T getValue() { return value; }
}

and modifying the Args class:

	public boolean getBoolean(char arg) {
		return process(Boolean.class, BooleanArgumentMarshaler.class, arg);
	}

	public String getString(char arg) {
		return process(String.class, StringArgumentMarshaler.class, arg);
	}

	public int getInt(char arg) {
		return process(Integer.class, IntegerArgumentMarshaler.class, arg);
	}

	public double getDouble(char arg) {
		return process(Double.class, DoubleArgumentMarshaler.class, arg);
	}

	public <S, T extends ArgumentMarshaler<S>> S process(Class<S> type, Class<T> marshalerClass, char arg) {
		ArgumentMarshaler m = marshalers.get(arg);

		if (m != null) {
			try {
				marshalerClass.cast(m);
				return marshalerClass.cast(m).getValue();
			}
			catch (ClassCastException e) { }
		}

		try {
			return marshalerClass.newInstance().getDefaultValue();
		}
		catch (InstantiationException e1) {}
		catch (IllegalAccessException e1) {}
		return null;
	}

(perhaps a bit complicated, right?)

Ugh, sorry, brain disconnect, of course you're already using generics

Are you conforming to any specific level of Java? If not, then I'd have thought that generics would be ideal to use for putting the getValue() method into the Marshaler interface. Or am I not seeing something?

OK. Will try and post a solution after working on it during the weekend.

ravi said:

The switch statement in ArgsException[?]? class's errorMessage method also makes change difficult. I generally have a Message class with a method that can handle messages with arbitrary number of parameters

I thought about that, but rejected it in this instance. The messages are more, or less, "helper" functions in case users need a shortcut to print an error message. Actually, (as I said in the article) I'm a bit ambivalent about having them there at all. It is arguably better simply to take the whole structure out and allow the users to create their own error messages. The only reason I left it in, is that I remember being grateful to authors who provide simple tools like that.

ravi also said:

Call me unimpressed with the cleanliness of the code that I have seen so far.

As you said, there is always room for improvement. I would like to see your complete solution. Would you mind posting it here?

The switch statement in ArgsException[?] class's errorMessage method also makes change difficult. I generally have a Message class with a method that can handle messages with arbitrary number of parameters, with the signature being:

static String Message.get(String messageCode, Object... parameters);

The messageCode is usually stored somewhere as a text file (or XML if you want.)
Using Java's MessageFormat.[?]format method makes my class easier to build and understand.

Instead of the switch statement in the code, I would have had something like:

public String errorMessage() {
return Message.get(this.errorCode, this.errorParameter);
}

I think this is much simpler than the code shown in the article.

It is not merely that Java is more verbose, it is the fact that Java developers at all levels do not build the tools that they need at the right level of abstraction. And this is the main reason that causes code bloat in Java.

Clean code? What I've seen so far has failed to impress me. Yes, it is of quality comparable to that seen in the better open source applications (Spring comes to mind), but there is room for improvement mainly by building the correct low-level abstractions and building the code on top of that.

In the first few pages of the article, I found this code:

public interface ArgumentMarshaler[?] {
void set(Iterator<String> currentArgument) throws ArgsException[?];
}

Now why would we have an interface with a method that has void return type? Any implementing class can do whatever it wants with it and still satisfy the interface requirements! But not necessarily the intent. And why are the getValue methods not a part of the ArgumentMarshaler[?] interface since they seem to be implemented by every concrete class?

I think that there is room for improvement here.

Furthermore, I see the if ... then ... else if ... construct in the method named parseSchemaElement. Again, that does not impress me.

Unfortunately Java does not have first class functions, and there is no "new" method. Else, we could get rid of these if .. then .. constructs and replace them with a method stored in a Map. Of course, one could also use newInstance() to generate the desired sub-class of ArgumentMarshaler[?] via a Factory approach to make the code easier to maintain. Using the Factory approach and the newInstance method, any additional sub-classes of ArgumentMarshaler[?] could be easily added without changing the parseSchemaElement method.

Call me unimpressed with the cleanliness of the code that I have seen so far.

I say that writing a book is like writing code. It's just the language that changes. You've been multilingual in this sense for decades. If you've been successful writing books and code in the past, why should it be any different now? Why question? Go with your gut and follow your bliss. If you're hesitating, then take that (gut) signal for what it is.

I somewhat understand, read, and speak more than my (one) native language. I've also written code in different languages. It's all about communication in the end. Clean Code is no more than Clear Communication. That which speaks clearest is obvious, intuitive, natural, and transcends even culture and time.... Sort of like arch-typing.

If you can write "clean" code, then you are no doubt able to write a "clean" book.... The source of the substance comes from the same place.

Considering how code intensive it will be, what about this format: left-hand page has code, right-hand page has commentary?