Album Cover

Refactoring C

Refactoring a Function

A module called Group has a function called getUsers(). Below are three versions of that function.
Please drag and drop the versions such that the best is on top, the 2nd best beneath it, and the worst is at the bottom.
  • Before you finish ordering, you can press the Show Hint button to reveal how many of your answers are right/wrong without specifying which are in the right/wrong position. Remember, you can continue to order items after hitting this button.
  • After you finish ordering, you can press the Check Answers button to see author commentary, as well as a green border for items in the right position and a red border for items in the wrong position.
Place these in the correct order: 
  • 
// Gets users sorted by the most recently registered user
UserList* getUsers(Group* group)
{
	int i = 0;
	FileList* files = NULL;
	UserList* users = newUserList();
    	if (!fileExists(persistencePath(group)))
		return users;
    	files = listFilesAt(persistencePath(group));
	for (i = 0; i != listSize(files); ++i)
	{
		File* file = (File*) listItemAt(i, files);
		if (file->isDirectory)
			listAddItem(newUser(file->name, group), users);
	}
	listSort(users, UserComparatorByDescendingRegistration);
    	return users;
}
    The comment at the top of this function is deodorant for code that does not quickly communicate what it does. You have to puzzle your way through the lines of code to figure out what it is doing. This is indeed the worst example of the function.
    (Reading...)
     
  • 
UserList* getUsers(Group* group)
{
	int i = 0;
	FileList* files = NULL;
	UserList* users = newUserList();
    	if (!userDirectoryExists(group))
		return users;
    	// Add found users to users collection
	files = listFilesAt(persistencePath(group));
	for (i = 0; i != listSize(files); ++i)
	{
		File* file = (File*) listItemAt(i, files);
		if (file->isDirectory)
	    	listAddItem(newUser(file->name, group), users);
	}
	// sort by most recently registered users
	listSort(users, UserComparatorByDescendingRegistration);
    	return users;
}
    This version uses comments to help clarify what the code does, instead of extracting code into well-named functions that communicate what each step of the function does.
    (Reading...)
     
  • 
UserList* getUsersSortedByMostRecentlyRegistered(Group* group)
{
	UserList* users = newUserList();
    	if (!userDirectoryExists(group))
		return users;
    	addFoundUsersTo(users, group);
	sortByMostRecentlyRegistered(users);
    	return users;
}
    This is indeed the best version. It is easy to understand how the function does its work, and there is no need for comments. Even the function name helps to communicate exactly what kind of data the function returns. This function is a great example of what self-documenting code can be.
    (Reading...)
       
(Reading...)
Page rating: Rate this Page
1 Poor
2 Needs Work
3 Acceptable
4 Cool
5 Awesome
Formatting Hints:
To format... use this markup:
source code {source:lang=java|lineNumbers=false}...{source}
(Supported languages: java, cpp, csharp, c, python.)
bold text __some text__
italicized text ~~some text~~
underlined text @@some text@@
a method or class name ``myMethod(...)``