I run across this type of code often (ASP.Net MVC)
[HttpPost]
public string Delete(ObjectId id)
{
var record = _Database.Get<Record>(id);
if (record == null)
{
return "No matching record!";
}
_Database.Remove<Record>(id);
return "Deleted record!";
}
If the user is interested in deleting the record, does it matter if someone else beat them to it?
[HttpPost]
public string Delete(ObjectId id)
{
_Database.Remove<Record>(id);
return "Deleted record!";
}
Isn't the HTTP 200 OK status code enough to indicate success, the user or the software knows the context of the request it made:
[HttpPost]
public void Delete(ObjectId id)
{
_Database.Remove<Record>(id);
}
Simple things to think about when reviewing code for simplicity, one line of code is much easier to maintain than seven.
When we develop libraries (whether internal or public), it helps to have a rapid ability to make changes and test them in a consuming application.
$projectSolution = 'src\\Library.sln' $nugetFeedPath = ENV["NuGetDevFeed"] msbuild :build => [:clean] do |msb| msb.properties :configuration => :Release msb.targets :Build msb.solution = $projectSolution end task :nuget => [:build] do sh "nuget pack src\\Library\\Library.csproj /OutputDirectory " + $nugetFeedPath end
If you manually execute any of this process, especially copying files, you will find it a burden to develop the library and will find yourself dreading it, and even worse, making changes downstream instead of updating the shared library for everyone’s sake.
@ECHO off echo Upload %1? set /P anykey="Hit enter to continue " nuget push %1 apikey set /P anykey="Done "
<li>run command to copy to public feed </li>
</ul>
Note: in this article when I refer to a commit, I mean the commit you plan to share with the rest of the team, if you have local commits that you plan to amend/combine, I am referring to the final result.
In time you will find these easier to do as you develop, however, all of these are valuable before checking in! The pre commit review is a nice time to polish what might have been several hours of intense work, during which these things were the last things on your mind! If you are concerned about losing your work in the process of responding to these questions, first do a check-in and amend it as you go (assuming you are using a tool such as git that supports this), rolling the result into one nice commit for everyone else.
In the show LOST, the Swan Station had a button that “had to be pushed” every 100 minutes to avoid disaster. Several characters in the show took it upon themselves to have faith and religiously push the button, resetting the clock and averting the unknown “disaster”. There are striking similarities in this story to the code we write every day. Here are some common ones that I encounter:
In all of these cases we haven’t done our due diligence to understand what the code we are writing is actually doing. In the rush to get things done it seems like we’re willing to push any button (add any line of code) just to get our desired result and move on. All of the above explanations are common things we encounter, and are valid ways to work through a problem we have, but when we find a solution to a task we are working on (whether a bug or a feature), we should take a moment to reflect on what we don’t understand. Remove what isn’t necessary, comprehend and simplify what is.
Why is it detrimental to commit code we don’t understand?
TLDR
Don’t commit code that you don’t understand, take the time to understand it, simplify it and then commit it!