Java by Comparison: Any tip about best practices?

[Adam Zubrycki]

How to evaluate effectiveness to learn by comparison? Any tip about best practices programming techniques will be valuable. What is the most important for Junior Java Developer with less than one-year hands-on experience?

[Simon Harrer]

Hi Adam,

we, as teachers, want to convince you to write you a specific way. So we have to make an argument. The best way to argue for something if you can see the effect at hand. Think about the ads of cleaning products. Before, the kitchen is dirty, but with the product X, you can clean it in no time, and, hey, look at how clean the kitchen now looks. :-) We do the same for clean code, advertising our clean code suggestions with the unique before/after code style, our "by comparison" style. We haven't evaluated it scientifically, but from the feedback we got throughout the years and for this book, we must do something right. :-)

We think our book is perfectly targeted for Junior Java Developers with less than one-year hands-on experience. I would recommend to read our book, and then go on reading "Clean Code" by Uncle Bob and "Effective Java 3rd Edition" by Joshua Bloch. And find a mentor that reviews your code! :-)

Best,
Simon

[Campbell Ritchie]

Simon Harrer wrote:. . . find a mentor that reviews your code! :-) . . .

We seem to do a lot of that on this website.

And, AZ, welcome to the Ranch

[Junilu Lacar]

Campbell Ritchie wrote:

Simon Harrer wrote:. . . find a mentor that reviews your code! :-) . . .

We seem to do a lot of that on this website.

I second that. In fact, I'd like to propose that we create a forum specifically for code reviews. This might be something that Simon, et. al. can consider sending their students to in the future to get feedback. They might post code for review or if that doesn't work for the instructors, post a link where anyone interested in providing feedback can go to do that.

What do you think, Simon?

[Junilu Lacar]

One thing I've learned here, having been involved in many threads where we've basically done code reviews and suggested alternative implementations, is that there are many ways to solve a problem and there may not be a single best way to do something. Also, what is considered "good" seems to be highly subjective at times. This is true in the industry as well and I think it would be good for students to experience that. What is good for one tech lead can be absolutely horrific to another tech lead. Often, it's not just a matter of aesthetics either. You will encounter various opinions about what exactly "clean code" is, despite the plethora of books and articles that try to explain it.

One recent discussion I had was around premature optimization and not doing performance tuning based on intuition alone. Some would argue that something like string concatenation vs. using a StringBuffer StringBuilder is an obvious choice. Others would argue that you should always have a quantitively justifiable reason to do any kind of performance tuning. Yet others may argue that performance tuning has nothing to do with clean code, that refactoring and optimization have different goals.

My point is that it may be useful for students to see different points of view. This might even become a source for interesting group or classroom discussions and can help them develop their critical thinking skills, another thing that does not have very good alignment between academic and industry expectations. I would be very interested in participating in these kind of discussions that students might have, weighing pros and cons of various perspectives on what is "clean code."

[Junilu Lacar]

The example code cited in this thread is a perfect example.

The original code was:

void log(String message, boolean classified) throws IOException {
   if (classified) {
      writeMessage(message, CAPTAIN_LOG);
   } else {
      writeMessage(message, CREW_LOG);
   }
}

and the suggested refactoring was:

void writeToCaptainLog(String message) throws IOException {
   writeMessage(message, CAPTAIN_LOG);
}

void writeToCrewLog(String message) throws IOException {
   writeMessage(message, CREW_LOG);
}

The sidebar was very appropriate: Remember: Good Design Is Difficult

This is true, because I would argue that the suggested refactoring actually changed the semantics of the call from the original. The naming used in the original code gives the impression that the logging was based on the message being "classified" or "not classified", hence the boolean parameter.

The new names introduced, however, totally eliminated those semantics and instead introduced the idea of "where" the message is being logged as the central idea around the variation.

I would argue that the following code is closer to the original code's intent and it doesn't create a leaky abstraction by revealing the implementation detail of where the message is actually logged.

void logClassified(String message) throws IOException {
   writeMessage(message, CAPTAIN_LOG);
}

void logUnclassified(String message) throws IOException {
   writeMessage(message, CREW_LOG);
}

[Junilu Lacar]

I would be very interested in hearing how the students would discuss the merits of one "expert" opinion versus another "expert" opinion.

This is where the critical thinking would come in and I think that would be a great learning experience.

[Junilu Lacar]

Simon, in addition to recommending Uncle Bob's "Clean Code" book and Josh Bloch's "Effective Java" book, please consider also recommending Corey Haines' "Understanding the Four Rules of Simple Design" -- it shows many different variations to solving the same problem with many detailed discussions on the motivations for choosing one design over another.  I learned a lot about design from this short (~70 pages) book and I highly recommend it to any aspiring and experienced programmer.

[Liutauras Vilda]

Junilu Lacar wrote:consider also recommending Corey Haines' "Understanding the Four Rules of Simple Design"

I can second that. When the first time I read it I thought: "fine, not bad, concise", when I read it second and third times after a year or so, I thought: "tremendous how many good lessons can be fitted to 70 pages".  Book has its charm, I find it to be very different from any other book, need some time to understand it and start loving it.

[Simon Harrer]

Thanks for the tip. Will have a look at that book for sure! Maybe we can recommend that in the next edition. :-)

Regarding your other code refactoring: hm, we should have named that boolean variable "useCaptainsLog" or something like that instead. Thanks for the hint. We'll try to get that fixed in the next edition. :-)

Best,
Simon

[Junilu Lacar]

Simon Harrer wrote:
Regarding your other code refactoring: hm, we should have named that boolean variable "useCaptainsLog" or something like that instead. Thanks for the hint. We'll try to get that fixed in the next edition. :-)

I disagree. The classified name makes more sense when you consider the values of CAPTAIN_LOG and CREW_LOG as implementation details. It was a good thing to hide those implementation details. Clients of the API could probably make a determination whether the message they wanted to log was classified or unclassified. In contrast, useCaptainsLog seems less intuitive. Why would you use the captain's log instead of the crew log?

[Simon Harrer]

I see. Probably an issue because we had too little space (pun :-) for the code.

Clients of the API should make the distinction whether the message is part of the classified captains log or the crew log. For us, those are domain concepts and not implementation details. And the captains log is classified as it happens.

But I agree - we should rewrite that item a little bit for the next edition. It's not as clear as it should be. Simply shows how hard it is to write those code snippets. :-)

Best,
Simon

[Junilu Lacar]

These kinds of discussions between experienced people can also be helpful for students trying to develop critical thinking skills. Please consider my idea of having a forum at CodeRanch specifically for code reviews that you can have your students participate in and get input from others.

[Simon Harrer]

Good idea. Will discuss this with the relevant people.

[Campbell Ritchie]

Junilu Lacar wrote:. . . The original code was:

void log(String message, boolean classified) throws IOException {
   if (classified) {
      writeMessage(message, CAPTAIN_LOG);
   } else {
      writeMessage(message, CREW_LOG);
   }
}

. . .

What about this?

void log(Message message) throws IOException {
   if (message.isClassified()) {
      writeMessage(message.getText(), CAPTAIN_LOG);
   } else {
      writeMessage(message.getText(),, CREW_LOG);
   }
}

I think the old version breaches the suggestion of Winston's that Strings Are Bad, and including the status in the Message object is more object‑oriented.

[Junilu Lacar]

Campbell Ritchie wrote:What about this?

void log(Message message) throws IOException {
   if (message.isClassified()) {
      writeMessage(message.getText(), CAPTAIN_LOG);
   } else {
      writeMessage(message.getText(), CREW_LOG);
   }
}

I think the old version breaches the suggestion of Winston's that Strings Are Bad, and including the status in the Message object is more object‑oriented.

I suppose that could be a design you could eventually refactor to. Refactoring should be done in small steps though. I could easily see the author's version being the first step, then you detect the smells that I pointed out and refactor to what I suggested. If that still seems smelly, you could certainly reify (message text + classification) into a Message class. If you want to take it a step even further, I'd make sure that the log method is the last publicly accessible API method and that writeMessage is a private method, at which point, I'd probably refactor to:

public final void log(Message message) throws LogException {
   if (message.isClassified()) {
       writeToClassifiedLog(message);
   } else {
       writeToUnclassifiedLog(message);
   }
}

private void writeToClassifiedLog(Message message) {
   try {
       captainLog.write(message.getText());
   } catch (IOException ioe) {
       throw new LogException("Error writing to captain's log", ioe);
   }
}

private void writeToUnclassifiedLog(Message message) {
   try {
       crewLog.write(message.getText());
   } catch (IOException ioe) {
       throw new LogException("Error writing to crew log", ioe);
   }
}

[Liutauras Vilda]

Adam Zubrycki, cowgratulations, your topic have been chosen to publish in CodeRanch's Journal April Edition