I am a never-nester, pursuing the ultimate aesthetics of simplicity and clarity, and I'm not alone.
Linus Torvalds, the inventor of the Linux kernel, shares my code aesthetics.
What is a never-nester? A never-nester never nests their code. We do have a "disgusting meter", which grows exponentially with every added tab.
Nesting code involves adding inner blocks to a function, and each open brace adds one level of depth to the function, allowing for greater complexity.
int calculate(int bottom, int top)
{
return bottom + top;
}This function has a depth of one because there are no inner blocks.
int calculate(int bottom, int top)
{
if (top > bottom)
{
return bottom + top;
}
else
{
return 0;
}
}By adding an if statement, we increase the depth of the function to two levels.
int calculate(int bottom, int top)
{
if (top > bottom)
{
int sum = 0;
for (int number = bottom; number <= top; number++)
{
sum += number;
}
return sum;
}
else
{
return 0;
}
}By adding a loop, we increase the depth of the function to three levels.
This is the maximum a never-nester can handle. A never-nester is reluctant to go beyond it.
Some of you may be curious to see what four levels of depth look like, and while it pains me to do so, I will demonstrate it for the sake of science.
int calculate(int bottom, int top)
{
if (top > bottom)
{
int sum = 0;
for (int number = bottom; number <= top; number++)
{
if (number % 2 == 0)
{
sum += number;
}
}
return sum;
}
else
{
return 0;
}
}By adding additional levels of depth, we have significantly increased the cognitive load required to comprehend this function. So, what can we do to mitigate this issue?
There are two helpful methods to de-nest:
Extraction
Inversion
Extraction
We can extract the inner part of the loop and turn it into a separate function.
int filterNumber(int number)
{
if (number % 2 == 0)
{
return number;
}
return 0;
}
int calculate(int bottom, int top)
{
if (top > bottom)
{
int sum = 0;
for (int number = bottom; number <= top; number++)
{
sum += filterNumber(number);
}
return sum;
}
else
{
return 0;
}
}Inversion
Nesting happy path code blocks deeper and deeper can create a lot of unnecessary complexity. Instead, we can invert the condition and put the unhappy path first.
First, we'll flip our if else statement by inverting the condition.
int filterNumber(int number)
{
if (number % 2 == 0)
{
return number;
}
return 0;
}
int calculate(int bottom, int top)
{
if (top < bottom)
{
return 0;
}
int sum = 0;
for (int number = bottom; number <= top; number++)
{
sum += filterNumber(number);
}
return sum;
}When you have a large number of conditions to check, such as the following:
void registerUser(String user)
{
String[] parts = user.split(":");
if (parts.length == 2)
{
int userId = Integer.parseInt(parts[0]);
if (userId >= 0)
{
String userName = parts[1];
if (users.containsKey(userId))
{
users.get(userId).setName(userName);
}
else
{
users.put(userId, new User(userName));
}
}
else
{
throw new IllegalArgumentException("Invalid user id: " + userId);
}
}
else
{
throw new IllegalArgumentException("Invalid user string: " + user);
}
}We can apply inversion repeatedly.
void registerUser(String user)
{
String[] parts = user.split(":");
if (parts.length != 2)
{
throw new IllegalArgumentException("Invalid user string: " + user);
}
int userId = Integer.parseInt(parts[0]);
if (userId < 0)
{
throw new IllegalArgumentException("Invalid user id: " + userId);
}
String userName = parts[1];
if (users.containsKey(userId))
{
users.get(userId).setName(userName);
}
else
{
users.put(userId, new User(userName));
}
}We end up with a validation gatekeeping section of the code, which declares the requirements of the function. You'll notice that the happy path moves down the function while all the error paths are indented.
When reading this code, I find that I can mentally discard the condition and focus on the core code. However, when it's nested, I find myself having to hold multiple ideas in my head simultaneously.
Let's examine a larger example:
public void run()
{
while (true)
{
String path;
while (path = requestedUrls.poll() != null)
{
downloads.add(new DownloadState(path));
}
if (!this.connectionDisabled)
{
ListIterator<DownloadState> iterator = downloads.listIterator();
while (iterator.hasNext() && !this.connectionDisabled)
{
DownloadState download = iterator.next();
if (download.getState() == DownloadState.State.Pending)
{
Download downloader = new Download(download.getUrl(), this.downloadDir);
downloader.start();
download.setDownloader(downloader);
download.moveTo(DownloadState.State.InProgress);
}
if (download.getState() == DownloadState.State.InProgress)
{
DownloadResult result = download.getDownloader().process();
switch(result.getCode())
{
case Success:
download.moveTo(DownloadState.State.Complete);
break;
case InProgress:
/* Nothing to do */
break;
case Timeout:
case ConnectionError:
if (download.getAttempts() > this.maxAttempts)
{
this.connectionDisabled = true;
}
else
{
download.moveTo(DownloadState.State.InProgress);
}
break;
case HttpError:
int HTTP_REQUEST_TIMEOUT = 408;
int HTTP_BAD_GATEWAY = 502;
int HTTP_SERVICE_UNAVAILABLE = 503;
int HTTP_GATEWAY_TIMEOUT = 504;
int httpCode = result.getHTTPCode();
if (httpCode == HTTP_REQUEST_TIMEOUT ||
httpCode == HTTP_BAD_GATEWAY ||
httpCode == HTTP_SERVICE_UNAVAILABLE ||
httpCode == HTTP_GATEWAY_TIMEOUT)
{
if (download.getAttemps() > this.maxAttemps)
{
download.moveTo(DownloadState.State.Complete);
}
else
{
download.moveTo(DownloadState.State.InProgress);
}
}
else
{
failedUrls.offer(download.getUrl());
download.moveTo(DownloadState.State.Complete);
}
break;
}
}
if (download.getState() == DownloadState.State.Complete)
{
iterator.remove();
}
}
}
if (this.connectionDisabled)
{
while (download.size() > 0)
{
DownloadState download = downloads.removeFirst();
if (download.getState() == DownloadState.State.InProgress)
{
download.getDownloader().cancel();
}
failedUrls.offer(download.getUrl());
}
}
if (downloads.isEmpty() || requestedUrls.isEmpty())
{
requestLock.lock();
try
{
newRequest.await();
}
catch(InterruptedException e)
{
return;
}
finally
{
requestLock.unlock();
}
}
}
}The purpose of this code is to download multiple files from the internet.
It talks with this download class that we cannot modify:
class Download
{
public Download(String url, String destination);
void start();
DownloadResult process();
public void cancel();
}It's an asynchronous download, which means that when we start the download, we have to call the process repeatedly. Each time we call it, we receive one of these results:
class DownloadResult
{
enum Code
{
Success,
InProgress,
ConnectionError,
Timeout,
HttpError
}
public Code getCode()
{
return Code.Success;
}
public int getHTTPCode()
{
return 200;
}
}Furthermore, you may want to download multiple files simultaneously in the background. To accomplish this, we have created a thread that manages all of the downloads.
ConcurrentLinkedQueue<String> requestedUrls == new ConcurrentLinkedQueue<String>();
ConcurrentLinkedQueue<String> failedUrls == new ConcurrentLinkedQueue<String>();
LinkedList<DownloadState> downloads = new LinkedList<DownloadState>();
Lock requestLock = new ReenTrantLock();
private Condition newRequest = requestLock.newCondition();
private boolean connectionDisabled = false;
public void appendDownload(String url)
{
requestLock.lock();
try
{
requestUrls.offer(url);
newRequest.signalAll();
}
finally
{
requestLock.unlock();
}
}New downloads enter the system through the "append download" method, which adds requested URLs to a queue.
The thread wakes up, retrieves the URLs from the queue, and adds them to a list of current downloads. Each download is assigned a state - pending, in progress, or complete - and on each cycle of the main loop, the thread checks the status of each download to determine the appropriate action.
If it's pending, we initiate a new download process.
If it's complete, we simply remove it from the list.
If it's in progress, we call the process method to monitor its status and determine the appropriate action.
If we encounter an error:
If the connection was successful, but we received an abnormal HTTP response, we evaluate whether the error is retriable. If it is, we retry up to three times and set the download status back to pending.
After numerous failed attempts, we will remove it from our download list and relocate it to a failure queue where someone else can handle it.
In the case of a connection error, we will attempt to retry up to three times. If the retries are unsuccessful, we will set a special "connection disabled" flag, which effectively causes us to abandon all downloads and clear the list.
Let's utilize extraction and inversion to flatten the structure.
To start, we can focus on the two main branches of download processing: "pending" and "in progress." Let's extract them out.
private void processPending(DownloadState download)
{
Download downloader = new Download(download.getUrl(), this.downloadDir);
downloader.start();
download.setDownloader(downloader);
download.moveTo(DownloadState.State.InProgress);
}
private void processInProgress(DownloadState download)
{
DownloadResult result = download.getDownloader().process();
switch(result.getCode())
{
case Success:
download.moveTo(DownloadState.State.Complete);
break;
case InProgress:
/* Nothing to do */
break;
case Timeout:
case ConnectionError:
if (download.getAttempts() > this.maxAttempts)
{
this.connectionDisabled = true;
}
else
{
download.moveTo(DownloadState.State.InProgress);
}
break;
case HttpError:
int HTTP_REQUEST_TIMEOUT = 408;
int HTTP_BAD_GATEWAY = 502;
int HTTP_SERVICE_UNAVAILABLE = 503;
int HTTP_GATEWAY_TIMEOUT = 504;
int httpCode = result.getHTTPCode();
if (httpCode == HTTP_REQUEST_TIMEOUT ||
httpCode == HTTP_BAD_GATEWAY ||
httpCode == HTTP_SERVICE_UNAVAILABLE ||
httpCode == HTTP_GATEWAY_TIMEOUT)
{
if (download.getAttemps() > this.maxAttemps)
{
download.moveTo(DownloadState.State.Complete);
}
else
{
download.moveTo(DownloadState.State.InProgress);
}
}
else
{
failedUrls.offer(download.getUrl());
download.moveTo(DownloadState.State.Complete);
}
break;
}
}
public void run()
{
while (true)
{
String path;
while (path = requestedUrls.poll() != null)
{
downloads.add(new DownloadState(path));
}
if (!this.connectionDisabled)
{
ListIterator<DownloadState> iterator = downloads.listIterator();
while (iterator.hasNext() && !this.connectionDisabled)
{
DownloadState download = iterator.next();
if (download.getState() == DownloadState.State.Pending)
{
processPending(dowload);
}
if (download.getState() == DownloadState.State.InProgress)
{
processInProgress(download);
}
if (download.getState() == DownloadState.State.Complete)
{
iterator.remove();
}
}
}
if (this.connectionDisabled)
{
while (download.size() > 0)
{
DownloadState download = downloads.removeFirst();
if (download.getState() == DownloadState.State.InProgress)
{
download.getDownloader().cancel();
}
failedUrls.offer(download.getUrl());
}
}
if (downloads.isEmpty() || requestedUrls.isEmpty())
{
requestLock.lock();
try
{
newRequest.await();
}
catch(InterruptedException e)
{
return;
}
finally
{
requestLock.unlock();
}
}
}
}The "in progress" function still seems too complicated to me. Perhaps we can simplify it further?
The HTTP error section is the most problematic. Let's move it out to improve the overall flow.
private void processPending(DownloadState download)
{
Download downloader = new Download(download.getUrl(), this.downloadDir);
downloader.start();
download.setDownloader(downloader);
download.moveTo(DownloadState.State.InProgress);
}
private handleHttpError(DownloadState download, int httpCode)
{
int HTTP_REQUEST_TIMEOUT = 408;
int HTTP_BAD_GATEWAY = 502;
int HTTP_SERVICE_UNAVAILABLE = 503;
int HTTP_GATEWAY_TIMEOUT = 504;
int httpCode = result.getHTTPCode();
if (httpCode == HTTP_REQUEST_TIMEOUT ||
httpCode == HTTP_BAD_GATEWAY ||
httpCode == HTTP_SERVICE_UNAVAILABLE ||
httpCode == HTTP_GATEWAY_TIMEOUT)
{
if (download.getAttemps() > this.maxAttemps)
{
download.moveTo(DownloadState.State.Complete);
}
else
{
download.moveTo(DownloadState.State.InProgress);
}
}
else
{
failedUrls.offer(download.getUrl());
download.moveTo(DownloadState.State.Complete);
}
}
private void processInProgress(DownloadState download)
{
DownloadResult result = download.getDownloader().process();
switch(result.getCode())
{
case Success:
download.moveTo(DownloadState.State.Complete);
break;
case InProgress:
/* Nothing to do */
break;
case Timeout:
case ConnectionError:
if (download.getAttempts() > this.maxAttempts)
{
this.connectionDisabled = true;
}
else
{
download.moveTo(DownloadState.State.InProgress);
}
break;
case HttpError:
handleHttpError(download, result.getHTTPCode());
break;
}
}
public void run()
{
while (true)
{
String path;
while (path = requestedUrls.poll() != null)
{
downloads.add(new DownloadState(path));
}
if (!this.connectionDisabled)
{
ListIterator<DownloadState> iterator = downloads.listIterator();
while (iterator.hasNext() && !this.connectionDisabled)
{
DownloadState download = iterator.next();
if (download.getState() == DownloadState.State.Pending)
{
processPending(dowload);
}
if (download.getState() == DownloadState.State.InProgress)
{
processInProgress(download);
}
if (download.getState() == DownloadState.State.Complete)
{
iterator.remove();
}
}
}
if (this.connectionDisabled)
{
while (download.size() > 0)
{
DownloadState download = downloads.removeFirst();
if (download.getState() == DownloadState.State.InProgress)
{
download.getDownloader().cancel();
}
failedUrls.offer(download.getUrl());
}
}
if (downloads.isEmpty() || requestedUrls.isEmpty())
{
requestLock.lock();
try
{
newRequest.await();
}
catch(InterruptedException e)
{
return;
}
finally
{
requestLock.unlock();
}
}
}
}As we continue with the "run" function, we can extract even further. The code can be divided into four major sections:
Processing incoming requests from the queue
Dealing with current downloads
Clearing out in-progress downloads
Waiting for a signal of new downloads to examine
void processPending(DownloadState download)
{
Download downloader = new Download(download.getUrl(), this.downloadDir);
downloader.start();
download.setDownloader(downloader);
download.moveTo(DownloadState.State.InProgress);
}
void handleHttpError(DownloadState download, int httpCode)
{
int HTTP_REQUEST_TIMEOUT = 408;
int HTTP_BAD_GATEWAY = 502;
int HTTP_SERVICE_UNAVAILABLE = 503;
int HTTP_GATEWAY_TIMEOUT = 504;
int httpCode = result.getHTTPCode();
if (httpCode == HTTP_REQUEST_TIMEOUT ||
httpCode == HTTP_BAD_GATEWAY ||
httpCode == HTTP_SERVICE_UNAVAILABLE ||
httpCode == HTTP_GATEWAY_TIMEOUT)
{
if (download.getAttemps() > this.maxAttemps)
{
download.moveTo(DownloadState.State.Complete);
}
else
{
download.moveTo(DownloadState.State.InProgress);
}
}
else
{
failedUrls.offer(download.getUrl());
download.moveTo(DownloadState.State.Complete);
}
}
boolean processInProgress(DownloadState download)
{
DownloadResult result = download.getDownloader().process();
switch(result.getCode())
{
case Success:
download.moveTo(DownloadState.State.Complete);
break;
case InProgress:
/* Nothing to do */
break;
case Timeout:
case ConnectionError:
if (download.getAttempts() > this.maxAttempts)
{
return true;
}
else
{
download.moveTo(DownloadState.State.InProgress);
}
break;
case HttpError:
handleHttpError(download, result.getHTTPCode());
break;
}
}
void processIncomingRequests()
{
String path;
while (path = requestedUrls.poll() != null)
{
downloads.add(new DownloadState(path));
}
}
boolean processDownload(ListIterator<DownloadState> iterator)
{
DownloadState download = iterator.next();
if (download.getState() == DownloadState.State.Pending)
{
processPending(dowload);
}
if (download.getState() == DownloadState.State.InProgress)
{
if (processInProgress(download))
{
return true;
}
}
if (download.getState() == DownloadState.State.Complete)
{
iterator.remove();
}
}
boolean processDownloads()
{
ListIterator<DownloadState> iterator = downloads.listIterator();
while (iterator.hasNext() && !this.connectionDisabled)
{
if (processDownload(iterator))
{
return true;
}
}
}
void clearDownloads()
{
while (download.size() > 0)
{
DownloadState download = downloads.removeFirst();
if (download.getState() == DownloadState.State.InProgress)
{
download.getDownloader().cancel();
}
failedUrls.offer(download.getUrl());
}
}
boolean waitForRequest()
{
requestLock.lock();
try
{
newRequest.await();
}
catch(InterruptedException e)
{
return false;
}
finally
{
requestLock.unlock();
}
return true;
}
public void run()
{
boolean running = true;
while (running)
{
processIncomingRequests();
if (!this.connectionDisabled)
{
this.connectionDisabled = processDownloads();
}
if (this.connectionDisabled)
{
clearDownloads();
}
if (downloads.isEmpty() || requestedUrls.isEmpty())
{
running = waitForRequest();
}
}
}Our main function now provides a clear outline of the steps, making the high-level logic easy to see.
According to the Linux kernel style guidelines, "If you need more than 3 levels of indentation, you're screwed anyway, and should fix your program."
I advocate this code aesthetics not only for its ability to make code structurally beautiful. More importantly, setting limits on the depth of nesting can encourage the creation of more efficient and readable code.
