The Eclipse Developer’s guide to Clean Code (part 1)

Front_pic“Even bad code can function. But if code isn’t clean, it can bring a development organization to its knees”  — Clean Code

We spent 10 times more time reading code than writing it. Thus keeping code clean is essential for maintainability and company growth, but doing it by hand can be tedious.

Let’s take a look at some of the clean code practices and how we can use Eclipse to re-factor code faster.

Change inline comments to sub method calls

In the land of in-line comments there are ‘good’ comments. These include things like showing ‘intend’ or ‘reason’ behind a decision. I.e they outline reasons that are not obvious from the code itself.
And there are comments that simply make a statement of what a block of code does. Those are considered ‘bad’ because of a number of reasons:

  1. It forces the reader to read both the comment and the code. (Too much effort if you ask me).
  2. Unclear comments can create ambiguity between the comment and the code.This adds additional thinking time. Only code is the truth.
  3. Code is often copied and pasted.  People sometimes forget to change the comment, so comments after time can lie and deceive.
  4. Code is eventually gets updated and people can forget to update the comments. Thus more lies and deception.

If you have a lengthy function and you use comments to annotate what the separate parts of the function does, then it would make sense to get rid of the comments and instead extract the code into helper methods. These helper methods become self-explanatory documentation.

But isn’t it too tedious to pull out code into sub-functions?

Not with Eclipse’s automatic method extraction. In fact most IDE’s have a notion of method-extraction. I use this feature dozens of times daily.
With a keyboard shortcut Eclipse can automatically create a helper function and replace the code with a call to that helper function. It automatically generates the method signature and find a nice placement for the function.
In fact Eclipse is smart enough to find duplicate code and replace that with a call to the helper function as well (i.e you get code deduplication).

Example of bad descriptive comments:

In the example below you can see that comments ‘state’ what code does and it there is a a lot of details.

public static boolean isOptionChecked(IProject project, String optionID, String parentToolName) {

//Get the active Configuration
IManagedBuildInfo buildInfo = ManagedBuildManager.getBuildInfo(project);
IConfiguration activeConf = buildInfo.getDefaultConfiguration();

//Get Compiler tool.
ITool[] tools = activeConf.getTools();
ITool gccCompileriTool = null;
for (ITool iTool : tools) {
if (iTool.getName().equals(parentToolName)) {
gccCompileriTool = iTool;
break;
}
}

//More code...
}

(Disclaimer: this is code I wrote before reading Clean Code).

To make it better, you can select some code and press Shift+Alt+m (or use the context menu).

Extract Method

You can type in the name of the helper function:
Extract Method 2

And see a preview of the new method:

Extract Method 3

As a result, we have much cleaner code. The first method is only 2 lines long.


public static boolean isOptionCheckedInCDT(IProject project, String optionIDString, String parentToolName) {
IConfiguration activeConfiguration = helperGetActiveConfiguration(project);
ITool gccCompileriTool = helperGetGccCompilerTool(parentToolName, activeConfiguration);
//more code.
}

private static IConfiguration helperGetActiveConfiguration(IProject project) {
IManagedBuildInfo buildInfo = ManagedBuildManager.getBuildInfo(project);
return buildInfo.getDefaultConfiguration();
}

private static ITool helperGetGccCompilerTool(String parentToolName, IConfiguration activeConf) {
ITool[] tools = activeConf.getTools();
ITool gccCompileriTool = null;
for (ITool iTool : tools) {
if (iTool.getName().equals(parentToolName)) {
gccCompileriTool = iTool;
break;
}
}
return gccCompileriTool;
}

Hide unnecessary details and deduplicate code.

Sometimes there are no in-line comments, never the less there is still more details in the code than necessary. There is the notion that a reader doesn’t need to know all the details of an implementation. Most often the reader just wants to find a specific section.  Having a large function with all the details is like a long Wikipedia article without any section headers or table of contents. It’s also hard to tell which variables are affected by which set of code, i.e our working memory is limited, there no need to buffer-overflow it with implementation details.
Further, when there is so much detail, it is hard to spot duplicate code. Thus it’s good to abstract away details by writing a sub-function(s) containing the details.

But how long should a function be?

According to Clean Code, a guiding principle is 3-4 lines.
This seems kind of ‘too idealistic’  in theory, but let us examine some examples.
I often find that 3-4 ‘concepts’ or notions is a better approximation. 3-4 variables or 3-4 independent sections.

Example of too much detail

Take this function from SWT GTK DateTime for instance. It’s a big blob of code and it’s hard to tell what it does at a glance. You have to read every line thoroughly, this is mental effort that is better spent elsewhere. In fact if you pay close attention there is duplicate code (can you spot it?):

void createHandle (int index) {
if ((style & SWT.CALENDAR) != 0) {
state |= HANDLE;
fixedHandle = OS.g_object_new (display.gtk_fixed_get_type (), 0);
if (fixedHandle == 0) error (SWT.ERROR_NO_HANDLES);
gtk_widget_set_has_window (fixedHandle, true);
handle = OS.gtk_calendar_new ();
if (handle == 0) error (SWT.ERROR_NO_HANDLES);
OS.gtk_container_add (fixedHandle, handle);
OS.gtk_calendar_set_display_options(handle, OS.GTK_CALENDAR_SHOW_HEADING | OS.GTK_CALENDAR_SHOW_DAY_NAMES);
} else {
fixedHandle = OS.g_object_new (display.gtk_fixed_get_type (), 0);
if (fixedHandle == 0) error (SWT.ERROR_NO_HANDLES);
gtk_widget_set_has_window (fixedHandle, true);
if ((style & SWT.DROP_DOWN) != 0 && (style & SWT.DATE) != 0) {
handle = OS.gtk_entry_new();
OS.gtk_container_add(fixedHandle, handle);
if (handle == 0) error (SWT.ERROR_NO_HANDLES);
} else {
long /*int*/ adjusment = OS.gtk_adjustment_new(0, -9999, 9999, 1, 0, 0);
handle = OS.gtk_spin_button_new(adjusment, 1, 0);
if (handle == 0) error (SWT.ERROR_NO_HANDLES);
OS.gtk_spin_button_set_numeric (handle, false);
OS.gtk_container_add(fixedHandle, handle);
OS.gtk_spin_button_set_wrap (handle, (style & SWT.WRAP) != 0);
}
OS.gtk_editable_set_editable (handle, (style & SWT.READ_ONLY) == 0);
OS.gtk_entry_set_has_frame (handle, (style & SWT.BORDER) != 0);

}
}

There is duplicate fixedHandle code.

Within about 2 minutes, a few Shfit+Alt+m presses later, the code is extracted to several sub-methods. Notice the first function is much shorter and easier to read. Eclipse also deduplicated the code for setting the fixedHandle.

void createHandle (int index) {
if ((style & SWT.CALENDAR) != 0) {
createHandleForCalendar();
} else { //DATE, TIME
setFixedHandle();
if ((style & SWT.DROP_DOWN) != 0 && (style & SWT.DATE) != 0) {
createHandleForDropDown();
} else {
createHandleForDateTime();
}
setWidgetProperties();
}
}

private void createHandleForCalendar() {
state |= HANDLE;
setFixedHandle();
handle = OS.gtk_calendar_new ();
if (handle == 0) error (SWT.ERROR_NO_HANDLES);
OS.gtk_container_add (fixedHandle, handle);
OS.gtk_calendar_set_display_options(handle, OS.GTK_CALENDAR_SHOW_HEADING | OS.GTK_CALENDAR_SHOW_DAY_NAMES);
}

private void setFixedHandle() {
fixedHandle = OS.g_object_new (display.gtk_fixed_get_type (), 0);
if (fixedHandle == 0) error (SWT.ERROR_NO_HANDLES);
gtk_widget_set_has_window (fixedHandle, true);
}

private void createHandleForDateTime() {
long /*int*/ adjusment = OS.gtk_adjustment_new(0, -9999, 9999, 1, 0, 0);
handle = OS.gtk_spin_button_new(adjusment, 1, 0);
if (handle == 0) error (SWT.ERROR_NO_HANDLES);
OS.gtk_spin_button_set_numeric (handle, false);
OS.gtk_container_add(fixedHandle, handle);
OS.gtk_spin_button_set_wrap (handle, (style & SWT.WRAP) != 0);
}

private void createHandleForDropDown() {
handle = OS.gtk_entry_new(); //LINFO Entry is created here for SWT.DROP_DOWN
OS.gtk_container_add(fixedHandle, handle);
if (handle == 0) error (SWT.ERROR_NO_HANDLES);
}

private void setWidgetProperties() {
OS.gtk_editable_set_editable (handle, (style & SWT.READ_ONLY) == 0);
OS.gtk_entry_set_has_frame (handle, (style & SWT.BORDER) != 0);
}

Part 2 coming soon…

Share

Leave a Reply