| |
Browse by Tags
All Tags » Rules (RSS)
-
Today, I'll do another installment of the CodeIt.Right
Rules, Explained series. This is post number five in the series. And,
as always, I'll start off by citing my two personal rules about static analysis guidance,
along with the explanation for them.
-
Never implement a suggested fix without knowing what makes it a fix.
-
Never ignore a suggested fix without understanding what makes it a fix.
It may seem as though I'm playing rhetorical games here. After all, I could
simply say, "learn the reasoning behind all suggested fixes." But I want to
underscore the decision you face when confronted with static analysis feedback.
In all cases, you must actively choose to ignore the feedback or address it.
And for both options, you need to understand the logic behind the suggestion.
In that spirit, I'm going to offer up explanations for three more CodeIt.Right rules
today.
Mark ISerializable Types with "Serializable" Attribute
If you run across this rule, you might do so while writing an exception class.
For example, the following small bit of code in a project of mine triggers it. public class GithubQueryingException
: Exception { public GithubQueryingException(string message,
Exception ex) : base(message,
ex) { } }
It seems pretty innocuous, right? Well, let's take a look at what went wrong.
The rule actually describes its own solution pretty well. Slap a serializable
attribute on this exception class and make the tool happy. But who cares?
Why does it matter if you don't mark the exception as serializable?
To understand the issue, you need awareness of a concept called "application
domains" within the .NET framework. Going into much detail about this would
take us beyond the scope of the post. But suffice it to say, "application domains
provide an isolation boundary for security, reliability, and versioning, and for unloading
assemblies." Think two separate processes running and collaborating.
If some external process will call your code, it won't access and deal with your objects
the same way that your own code will. Instead, it needs to communicate by serializing
the object and passing it along as if over some remote service call. In the
case of the exception above, it lacks the attribute marking it explicitly as serializable,
in spite of implementing that interface. So bad things will happen at runtime.
And this warning exists to give you the heads up.
If you'll only ever handle this exception within the same app domain, it won't cause
you any heartburn. But, then again, neither will adding an attribute to your
class.
Do Not Handle Non-CLS-Compliant Exceptions
Have you ever written code that looks something like this? try {
DoSomething(); return true;
} catch { return false;
}
In essence, you want to take a stab at doing something and return true if it goes
well and false if anything goes wrong. So you write code that looks something
like the above.
If you you have, you'll run afoul of the CodeIt.Right rule, "do not handle non-cls-compliant
exceptions." You might find this confusing at first blush, particularly if you
code exclusively in C# or Visual Basic. This would confuse you because you cannot
throw exceptions not compliant with the common language specification (CLS).
All exceptions you throw inherit from the Exception class and thus conform.
However, in the case of native code written in, say, C++, you can actually
throw non-CLS-compliant exceptions. And this code will catch them because you've
said "catch anything that comes my way." This earns you a warning.
The CodeIt.Right warning here resembles one telling you not to catch the general exception
type. You want to be intentional about what exceptions you trap, rather than
casting an overly wide net. You can fix this easily enough by specifying the
actual exception you anticipate might occur.
Async Methods Should Return Task or Task<T>
As of .NET Framework 4.5, you can use the async
keyword to allow invocation of an asynchronous operation. For example, imagine
that you had a desktop GUI app and you wanted to populate a form with data.
But imagine that acquiring said data involved doing an expensive and time consuming
call over a network.
With synchronous programming, the call out to the network would block, meaning
that everything else would grind to a halt to wait on the network call... including
the GUI's responsiveness. That makes for a terrible user experience. Of
course, we solved this problem long before the existence of the async keyword.
But we used laborious threading solutions to do that, whereas the async keyword makes
this more intuitive.
Roughly speaking, designating a method as "async" indicates that you can dispatch
it to conduct its business while you move on to do other things. To accomplish
this, the method synchronously returns something called a Task, which acts as a placeholder
and a promise of sorts. The calling method keeps a reference to the Task and
can use it to get at the result of the method, once the asynchronous operation completes.
But that only works if you return a Task or Task<T>. If, instead, you
create a void method and label it asynchronous, you have no means to get at it later
and no means to explicitly wait on it. There's a good chance this isn't what
you want to do, and CodeIt.Right lets you know that. In the case of an event
handler, you might actually want to do this, but better safe than sorry. You
can fix the violation by returning a non-parameterized Task rather than declaring
the method void.
Until Next Time
This post covered some interesting language and framework features. We looked
at the effect of crossing app domain boundaries and what that does to the objects
whose structure you can easily take for granted. Then we went off the beaten
path a little by looking at something unexpected that can happen at the intersection
of managed and native code. And, finally, we delved into asynchronous programming
a bit.
As we wander through some of these relatively far-reaching concerns, it's nice to
see that CodeIt.Right helps
us keep track. A good analysis tool not only helps you catch mistakes, but it
also helps you expand your understanding of the language and framework.
Learn
more how CodeIt.Right can help you automate code reviews and improve your code quality.
About the Author
Erik Dietrich
I'm a passionate software developer and active blogger. Read about me at my
site. View
all posts by Erik Dietrich
|
-
"You never concatenate strings. Instead, always use a StringBuilder."
I feel pretty confident that any C# developer that has ever worked in a group has
heard this admonition at least once. This represents one of those bits of developer
wisdom that the world expects you to just memorize. Over the course of
your career, these add up. And once they do, grizzled veterans engage in a sort
of comparative jousting for rank. The internet
encourages them and eggs them on.
"How can you call yourself a senior C# developer and not know how to serialize
objects to XML?!"
With two evenly matched veterans swinging language swords at one another, this volley
may continue for a while. Eventually, though, one falters and pecking order
is established.
Static Analyzers to the Rescue
I must confess. I tend to do horribly at this sort of thing. Despite having
relatively good memory retention ability in theory, I have a critical Achilles Heel
in this regard. Specifically, I can only retain information that interests me.
And building up a massive arsenal of programming language "how-could-yous" for dueling
purposes just doesn't interest me. It doesn't solve any problem that
I have.
And, really, why should it? Early in my career, I figured out the joy of static
analyzers in pretty short order. Just as the ubiquity of search engines means
I don't need to memorize algorithms, the presence of static analyzers saves me from
cognitively carrying around giant checklists of programming sins to avoid. I
rejoiced in this discovery. Suddenly, I could solve interesting problems
and trust the equivalent of programmer spell check to take care of the boring stuff.
Oh, don't get me wrong. After the analyzers slapped me, I internalized the lessons.
But I never bothered to go out of my way to do so. I learned only in response
to an actual, immediate problem. "I don't like seeing warnings, so let me figure
out the issue and subsequently avoid it."
My Coding Provincialism
This general modus operandi caused me to respond predictably when I first encountered
the idea of globalization in language. "Wait, so this helps when? If someone
theoretically deploys code to some other country? And, then, they might see
dates printed in a way that seems strange to them? Huh."
For many years, this solved no actual problem that I had. Early in my career,
I wrote software that people deployed in the US. Much of it had no connectivity
functionality. Heck, a lot of it didn't even have a user interface. Worst
case, I might later have to realize that some log file's time stamps happened in Mountain
Time or something.
Globalization solved no problem that I had. So when I heard rumblings about
the "best practice," I generally paid no heed. And, truth be told, nobody suffered.
With the software I wrote for many years, this would have constituted a premature
optimization.
But it nevertheless instilled in me a provincialism regarding code.
A Dose of Reality
I've spent my career as a polyglot. And so at one point, I switched jobs, and
it took me from writing Java-based web apps to a desktop app using C# and WPF.
This WPF app happened to have worldwide distribution. And, when I say worldwide,
I mean just about every country in the world.
Suddenly, globalization went from "premature optimization" to "development table stakes."
And the learning curve became steep. We didn't just need to account for
the fact that people might want to see dates where the day, rather than the month,
came first. The GUI needed translation into dozens of languages as a menu setting.
This included languages with text read from right to left.
How did I deal with this? At the time, I don't recall having the benefit of
a static analyzer that helped in this regard. FXCop may have provided some relief,
but I don't recall one way or the other. Instead, I found myself needing to study and
laboriously create mental checklists. This "best practice" knowledge hoarding
suddenly solved an immediate problem. So, I did it.
CodeIt.Right's Globalization Features
Years have passed since then. I've had several jobs since then, and, as a solo
consultant, I've had dozens of clients and gigs. I've lost my once encyclopedic
knowledge of globalization concerns. That happened because -- you guessed it
-- it no longer solves an immediate problem that I have.
Oh, I'd probably do better with it now than I did in the past. But I'd still
have to re-familiarize myself with the particulars and study up once again in order
to get it right, should the need arise. Except, these days, I could enlist
some help. CodeIt.Right,
installed on my machine, will give me the heads up I didn't have those years ago.
It has a number of globalization concerns built right in. Specifically, it will
remind you about the following concerns. I'll just list them here, saving detailed
explanations for a future "CodeIt.Right Rules, Explained" post.
-
Specify culture info
-
Specify string comparison (for culture)
-
Do not pass literals as localized parameters
-
Normalize strings to uppercase
-
Do not hard code locale specific strings
-
Use ordinal string comparison
-
Specify marshaling for PInvoke string arguments
-
Set locale for data types
That provides an excellent head start on getting savvy with globalization.
The Takeaway
Throughout the post, I've talked about my tendency not to bother with things that
don't solve immediate problems for me. I realize philosophical differences in
approach exist, but I stand by this practice to this day. And I don't say this
only because of time savings and avoiding premature optimization. Storing up
an arsenal of specific "best practices" in your head threatens to entrench you in
your ways and to establish an approach of "that's just how you do it."
And yet, not doing this can lead to making rookie mistakes and later repeating them.
But, for me, that's where automated tooling enters the picture. I understand
the globalization problem in theory. That I have not forgotten.
And I can use a tool like CodeIt.Right to
bridge the gap between theory and specifics in short order, creating just-in-time
solutions to problems that I have.
So to conclude the post, I would offer the following in takeaway. Stop memorizing
all of the little things you need to check for at the method level in coding. Let
tooling do that for you, so that you can keep big picture ideas in your head.
I'd say, "don't lose sight of the forest for the trees," but with tooling, you can
see the forest and the trees.
Learn
more how CodeIt.Right can help you automate code reviews, improve your code quality,
and ensure your code is globalization ready.
About the Author
Erik Dietrich
I'm a passionate software developer and active blogger. Read about me at my
site. View
all posts by Erik Dietrich
|
-
Today, I'll do another installment of the CodeIt.Right
Rules, Explained series. I have now made four such posts in this series.
And, as always, I'll start off by citing my two personal rules about static analysis
guidance.
-
Never implement a suggested fix without knowing what makes it a fix.
-
Never ignore a suggested fix without understanding what makes it a fix.
It may seem as though I'm playing rhetorical games here. After all, I could
simply say, "learn the reasoning behind all suggested fixes." But I want to
underscore the decision you face when confronted with static analysis feedback.
In all cases, you must actively choose to ignore the feedback or address it.
And for both options, you need to understand the logic behind the suggestion.
In that spirit, I'm going to offer up explanations for three more CodeIt.Right rules
today.
Type that contains only static members should be sealed
Let's start here with a quick example. I think this picture will suffice for
some number of words, if not necessarily one thousand.
Here, I've laid a tiny seed for a Swiss Army Knife, "utils" class. Presumably,
I will continue to dump any method I think might help me with Linq into this class.
But for now, it contains only a single method to make things easy to understand.
(As an aside, I discourage "utils" classes as a practice. I'm using this example
because everyone reading has most assuredly seen one of these things at some point.)
When you run CodeIt.Right analysis on this code, you will find yourself confronted
with a design issue. Specifically, "types that contain only static members should
be sealed."
You probably won't have a hard time discerning how to remedy the situation.
Adding the "sealed" modifier to the class will do the trick. But why does CodeIt.Right
object?
The Microsoft
guidelines contain a bit more information. They briefly explain that static
analyzers make an inference about your design intent, and that you can better communicate
that intent by using the "sealed" keyword. But let's unpack that a bit.
When you write a class that has nothing but static members, such as a static utils
class, you create something with no instantiation logic and no state. In other
words, you could instantiate "a LinqUtils," but you couldn't do anything
with it. Presumably, you do not intend that people use the class in that way.
But what about other ways of interacting with the class, such as via inheritance?
Again, you could create a LinqUtilsChild that inherited from LinqUtils, but
to what end? Polymorphism requires instance members, and non exist here.
The inheriting class would inherit absolutely nothing from its parent, making the
inheritance awkward at best.
Thus the intent of the rule. You can think of it telling you the following.
"You're obviously not planning to let people use inheritance with you, so don't even
leave that door open for them to possibly make a mistake."
So when you find yourself confronted with this warning, you have a simple bit of consideration.
Do you intend to have instance behavior? If so, add that behavior and the warning
goes away. If not, simply mark the class sealed.
Async methods should have async suffix
Next up, let's consider a rule in the naming category. Specifically, when you
name an async method with suffixing "async" on its name, you see the warning.
Microsoft declares
this succinctly in their guidelines.
By convention, you append "Async" to the names of methods that have an async modifier.
So, CodeIt.Right simply tells us that we've run afoul of this convention. But,
again, let's dive into the reasoning behind this rule.
When Microsoft introduced this programming paradigm, they did so in a non-breaking
release. This caused something of a conundrum for them because of a perfectly
understandable language rule stating that method overloads cannot vary only by a return
type. To take advantage of the new language feature, users would need to offer
the new, async methods, and also backward compatibility with existing method calls.
This put them in the position of needing to give the new, async methods different
names. And so Microsoft offered guidance on a convention for doing so.
I'd like to make a call-out here with regard to my two rules at the top of each post.
This convention came about because of expediency and now sticks around for convention's
sake. But it may bother you that you're asked to bake a keyword into the name
of a method. This might trouble you in the same way that a method called "GetCustomerNumberString()"
might bother you. In other words, while I don't advise you go against convention,
I will say that not all warnings are created equally.
Always define a global error handler
With this particular advice, we dive into warnings specific to ASP. When you
see this warning, it concerns the Global.asax file. To understand a bit more
about that, you can
read this Stack Overflow question. In short, Global.asax allows you to define
responses to "system level" in a single place.
CodeIt.Right is telling you to define just such an event -- specifically one in response
to the "Application_Error" event. This event occurs whenever an exception bubbles
all the way up without being trapped anywhere by your code somewhere. And, that's
a perfectly reasonable state of affairs -- your code won't trap every possible
exception.
CodeIt.Right wants you to define a default behavior on application errors. This
could mean something as simple as redirecting to a page that says, "oops, sorry about
that." Or, it could entail all sorts of robust, diagnostic information.
The important thing is that you define it and that it be consistent.
You certainly don't want to learn from your users what your own application does in
response to an error.
So spent a bit of time defining your global error handling behavior. By all
means, trap and handle exceptions as close to the source as you can. But always
make sure to have a backup plan.
Until Next Time
In this post, I ran the gamut across concerns. I touched on an object-oriented
design concern. Then, I went into a naming consideration involving async, and,
finally, I talked specifically about ASP programming considerations.
I don't have a particular algorithm for the order in which I cover these subjects.
But, I like the way this shook out. It goes to show you that CodeIt.Right covers
a lot of ground, across a lot of different landscapes of the .NET world.
Learn
more how CodeIt.Right can help you automate code reviews and improve your code quality.
About the Author
Erik Dietrich
I'm a passionate software developer and active blogger. Read about me at my
site. View
all posts by Erik Dietrich
|
-
In what has become a
series of posts, I have been explaining some CodeIt.Right rules in depth.
As with the last post in the series, I'll start off by citing two rules that I, personally,
follow when it comes to static code analysis.
-
Never implement a suggested fix without knowing what makes it a fix.
-
Never ignore a suggested fix without understanding what makes it a fix.
It may seem as though I'm playing rhetorical games here. After all, I could
simply say, "learn the reasoning behind all suggested fixes." But I want to
underscore the decision you face when confronted with static analysis feedback.
In all cases, you must actively choose to ignore the feedback or address it.
And for both options, you need to understand the logic behind the suggestion.
In that spirit, I'm going to offer up explanations for three more CodeIt.Right rules
today.
Use Constants Where Appropriate
First up, let's consider the admonition to "use constants where appropriate."
Consider this code that I lifted from a
Github project I worked on once.
I received this warning on the first two lines of code for this class. Specifically,
CodeIt.Right objects to my usage of static readonly string. If I let
CodeIt.Right fix the issue for me, I wind up with the following code.
Now, CodeIt.Right seems happy. So, what gives? Why does this matter?
I'll offer you the
release notes of the version where CodeIt.Right introduced this rule. If
you look at the parenthetical next to the rule, you will see "performance."
This preference has something to do with code performance. So, let's get specific.
When you declare a variable using const or static readonly,
think in terms of magic values and their elimination. For instance, imagine
my UserAgentKey value. Why do you think I declare that the way
I did? I did it to name that string, rather than using it inline as a "magic"
string.
As a maintenance programmer, how frustrating do you find stumbling across lines of
code like, "if(x == 299)"? "What is 299, and why do we care?!"
So you introduce a variable (or, preferably, a constant) to document your intent.
In the made-up hypothetical, you might then have "if(x == MaximumCountBeforeRetry)".
Now you can easily understand what the value means.
Either way of declaring this (constant or static, readonly field) serves the replacement
purpose. In both cases, I replace a magic value with a more readable, named
one. But in the case of static readonly, I replace it with a variable,
and in the case of const, I replace it with, well, a const.
From a performance perspective, this matters. You can think of a declaration
of const as simply hard-coding a value, but without the magic. So, when I switch
to const, in my declaration, the compiler replaces every version of UserAgentKey with
the string literal "user-agent". After compilation, you can't tell whether I
used a const or just hard-coded it everywhere.
But with a static readonly declaration, it remains a variable, even when
you use it like a constant. It thus incurs the relative overhead penalty of
performing a variable lookup at runtime. For this reason, CodeIt.Right steers
you toward considering making this a constant.
Parameter Names Should Match Base Declaration
For the next rule, let's return to the Github scraper project from the last example.
I'll show you two snippets of code. The first comes from an interface definition
and the second from a class implementing that interface. Pay specific attention
to the method, GetRepoSearchResults.
If you take a look at the parameter names, it probably won't surprise you to see that
they do not match. Therein lies the problem that CodeIt.Right has with my code.
It wants the implementing class to match the interface definition (i.e. the "base").
But why?
In this case, we have a fairly simple answer. Having different names for the
conceptually same method creates confusion.
Specifically, maintainers will struggle to understand whether you meant to override
or overload the method. In our mind's eyes, identical method signatures signals
polymorphic approaches, while same name, different parameters signals overload.
In a sense, changing the name of a variable fakes maintenance programmers out.
Do Not Declare Externally Visible Instance Fields
I don't believe we need a screenshot for this one. Consider the following trivial
code snippet.
public class SomeClass
{ public string _someVariable;
}
This warning says, "don't do that." More specifically, don't declare an instance
field with external (to the type) visibility. The question is, "why not?"
If you check out the Microsoft guidance on the subject, they explain that, the "use
of a field should be as an implementation detail." In other words, they contend
that you violate encapsulation by exposing fields. Instead, they say, you should
expose this via a property (which simply offers syntactic sugar over a method).
Instead of continuing with abstract concepts, I'll offer a concrete example.
Imagine that you want to model a family and you declare an integer field called _numberOfChildren.
That works fine initially, but eventually you encounter the conceptually weird edge
case where someone tries to define a family with -1 children. With an integer
field, you can technically do this, but you want to prevent that from happening.
With clients of your class directly accessing and setting this field, you wind up
having to go install this guard logic literally everywhere your clients interact with
the field. But had you hidden the field behind a property, you could simply
add logic to the property setter wherein you throw an exception on an attempt to set
a negative value.
This rule attempts to help you future-proof your code and follow good OO practice.
Until Next Time
Somewhat by coincidence, this post focused heavily on the C# flavor of object-oriented
programming. We looked at constants versus field access, but then focused on
polymorphism and encapsulation.
I mention this because I find it interesting to see where static analyzers take you.
Follow along for the rest of the series and, hopefully, you'll learn various useful
nuggets about the language you use.
Learn
more how CodeIt.Right can help you automate code reviews and improve your code quality.
About the Author
Erik Dietrich
I'm a passionate software developer and active blogger. Read about me at my
site. View
all posts by Erik Dietrich
|
-
A little while back, I started
a post series explaining some of the CodeIt.Right rules. I led into the
post with a narrative, which I won't retell. But I will reiterate the two rules
that I follow when it comes to static analysis tooling.
-
Never implement a suggested fix without knowing what makes it a fix.
-
Never ignore a suggested fix without understanding what makes it a fix.
Because I follow these two rules, I find myself researching every fix suggested to
me by my tooling. And, since I've gone to the trouble of doing so, I'll save
you that same trouble by explaining some of those rules today. Specifically,
I'll examine 3 more CodeIt.Right rules
today and explain the rationale behind them.
Mark assemblies CLSCompliant
If you develop in .NET, you've no doubt run across this particular warning at some
point in your career. Before we get into the details, let's stop and define
the acronyms. "CLS" stands for "Common Language Specification," so the warning
informs you that you need to mark your assemblies "Common Language Specification Compliant"
(or non-compliant, if applicable).
Okay, but what does that mean? Well, you can easily forget that many programming
languages target the .NET runtime besides your language of choice. CLS compliance
indicates that any language targeting the runtime can use your assembly. You
can write language specific code, incompatible with other framework languages.
CLS compliance means you haven't.
Want an example? Let's say that you write C# code and that you decide to get
cute. You have a class with a "DoStuff" method, and you want to add a slight
variation on it. Because the new method adds improved functionality, you decide
to call it "DOSTUFF" in all caps to indicate its awesomeness. No problem, says
the C# compiler.
And yet, if you you try to do the same thing in Visual Basic, a case insensitive language,
you will encounter a compiler error. You have written C# code that VB code cannot
use. Thus you have written non-CLS compliant code. The CodeIt.Right rule
exists to inform you that you have not specified your assembly's compliance or non-compliance.
To fix, go specify. Ideally, go into the project's AssemblyInfo.cs file and
add the following to call it a day.
[assembly:CLSCompliant(true)]
But you can also specify non-compliance for the assembly to avoid a warning.
Of course, you can do better by marking the assembly compliant on the whole and then
hunting down and flagging non-compliant methods with the attribute.
Specify IFormatProvider
Next up, consider a warning to "specify IFormatProvider." When you encounter
this for the first time, it might leave you scratching your head. After all,
"IFormatProvider" seems a bit... technician-like. A more newbie-friendly name
for this warning might have been, "you have a localization problem."
For example, consider a situation in which some external supplies a date. Except,
they supply the date as a string and you have the task of converting it to a proper DateTime so
that you can perform operations on it. No problem, right?
var properDate = DateTime.Parse(inputString);
That should work, provided provincial concerns do not intervene. For those of
you in the US, "03/02/1995" corresponds to March 2nd, 1995. Of course, should
you live in Iraq, that date string would correspond to February 3rd, 1995. Oops.
Consider a nightmare scenario wherein you write some code with this parsing mechanism.
Based in the US and with most of your customers in the US, this works for years.
Eventually, though, your sales group starts making inroads elsewhere. Years
after the fact, you wind up with a strange bug in code you haven't touched for years.
Yikes.
By specifying a format provider, you can avoid this scenario.
Nested types should not be visible
Unlike the previous rule, this one's name suffices for description. If you declare
a type within another type (say a class within a class), you should not make the nested
type visible outside of the outer type. So, the following code triggers the
warning.
public class Outer
{ public class Nested
{ } }
To understand the issue here, consider the object oriented principle of encapsulation.
In short, hiding implementation details from outsiders gives you more freedom to vary
those details later, at your discretion. This thinking drives the rote instinct
for OOP programmers to declare private fields and expose them via public accessors/mutators/properties.
To some degree, the same reasoning applies here. If you declare a class or struct inside
of another one, then presumably only the containing type needs the nested one.
In that case, why make it public? On the other hand, if another type does, in
fact, need the nested one, why scope it within a parent type and not just the same
namespace?
You may have some reason for doing this -- something specific to your code and your
implementation. But understand that this is weird, and will tend to create awkward,
hard-to-discover code. For this reason, your static analysis tool flags your
code.
Until Next Time
As I said last time, you can extract a ton of value from understanding code analysis
rules. This goes beyond just understanding your tooling and accepted best practice.
Specifically, it gets you in the habit of researching and understanding your code
and applications at a deep, philosophical level.
In this post alone, we've discussed language interoperability, geographic maintenance
concerns, and object oriented design. You can, all too easily, dismiss analysis
rules as perfectionism. They aren't; they have very real, very important applications.
Stay tuned for more posts in this series, aimed at helping you understand your tooling.
Learn
more how CodeIt.Right can help you automate code reviews and improve your code quality.
About the Author
Erik Dietrich
I'm a passionate software developer and active blogger. Read about me at my
site. View
all posts by Erik Dietrich
|
-
I've heard tell of a social experiment conducted with monkeys. It may or may
not be apocryphal, but it illustrates an interesting point. So, here goes.
Primates and Conformity
A group of monkeys inhabited a large enclosure, which included a platform in the middle,
accessible by a ladder. For the experiment, their keepers set a banana on the
platform, but with a catch. Anytime a monkey would climb to the platform, the
action would trigger a mechanism that sprayed the entire cage with freezing cold water.
The smarter monkeys quickly figured out the correlation and actively sought to prevent
their cohorts from triggering the spray. Anytime a monkey attempted to climb
the ladder, they would stop it and beat it up a bit by way of teaching a lesson.
But the experiment wasn't finished.
Once the behavior had been established, they began swapping out monkeys. When
a newcomer arrived on the scene, he would go for the banana, not knowing the social
rules of the cage. The monkeys would quickly teach him, though. This continued
until they had rotated out all original monkeys. The monkeys in the cage would
beat up the newcomers even though they had never experienced the actual negative
consequences.
Now before you think to yourself, "stupid monkeys," ask yourself how much better you'd
fare. This
video shows that humans have the same instincts as our primate cousins.
Static Analysis and Conformity
You might find yourself wondering why I told you this story. What does it have
to do with software tooling and static analysis?
Well, I find that teams tend to exhibit two common anti-patterns when it comes to
static analysis. Most prominently, they tune out warnings without due diligence.
After that, I most frequently see them blindly implement the suggestions.
I tend to follow two rules when it comes to my interaction with static analysis tooling.
-
Never implement a suggested fix without knowing what makes it a fix.
-
Never ignore a suggested fix without understanding what makes it a fix.
You syllogism buffs out there have, no doubt, condensed this to a single rule.
Anytime you encounter a suggested fix you don't understand, go learn about it.
Once you understand it, you can implement the fix or ignore the suggestion with eyes
wide open. In software design/architecture, we deal with few clear cut rules
and endless trade-offs. But you can't speak intelligently about the trade-offs
without knowing the theory behind them.
Toward that end, I'd like to facilitate that warning for some CodeIt.Right rules
today. Hopefully this helps you leverage your tooling to its full benefit.
Abstract types should not have public constructors
First up, consider the idea of abstract types with public constructors.
public abstract class Shape
{ protected ConsoleColor
_color; public Shape(ConsoleColor
color) { _color = color;
} } public class Square
: Shape { public int SideLength
{ get; set;
} public Square(ConsoleColor
color) : base(color)
{ } }
CodeIt.Right will ding you for making the Shape constructor public (or
internal -- it wants protected). But why?
Well, you'll quickly discover that CodeIt.Right has good company in the form of the
.NET Framework guidelines and FxCop rules. But that just shifts the discussion
without solving the problem. Why does everyone seem not to like this
code?
First, understand that you cannot instantiate Shape, by design. The "abstract"
designation effectively communicates Shape's incompleteness. It's more of a template than
a finished class in that creating a Shape makes no sense without the added specificity
of a derived type, like Square.
So the only way classes outside of the inheritance hierarchy can interact with Shape
indirectly, via Square. They create Squares, and those Squares decide how to
go about interacting with Shape. Don't believe me? Try getting around
this. Try creating a Shape in code or try deleting Square's constructor and
calling new Square(color). Neither will compile.
Thus, when you make Shape's constructor public or internal, you invite users of your
inheritance hierarchy to do something impossible. You engage in false
advertising and you confuse them. CodeIt.Right is helping you avoid this
mistake.
Do not catch generic exception types
Next up, let's consider the wisdom, "do not catch generic exception types."
To see what that looks like, consider the following code.
public bool MergeUsers(int user1Id, int user2Id)
{ try { var user1 = _userRepo.Get(user1Id); var user2 = _userRepo.Get(user2Id);
user1.MergeWith(user2); _userRepo.Save(user1); _userRepo.Delete(user2); return true;
} catch(Exception
ex) { _logger.Log($"Exception
{ex.Message} occurred."); return false;
} }
Here we have a method that merges two users together, given their IDs. It accomplishes
this by fetching them from some persistence ignorance scheme, invoking a merge operation,
saving the merged one and deleting the vestigial one. Oh, and it wraps the whole
thing in a try block, and then logs and returns false should anything fail.
And, by anything, I mean absolutely anything. Business rules make merge
impossible? Log and return false. Server out of memory? Log it and
return false. Server hit by lightning and user data inaccessible? Log
it and return false.
With this approach, you encounter two categories of problem. First, you fail
to reason about or distinguish among the different things that might go wrong.
And, secondly, you risk overstepping what you're equipped to handle here. Do
you really want to handle fatal system exceptions right smack in the heart
of the MergeUsers business logic?
You may encounter circumstances where you want to handle everything, but probably
not as frequently as you think. Instead of defaulting to this catch all, go
through the exercise of reasoning about what could go wrong here and what you want
to handle.
Avoid language specific type names in parameters
If you see this violation, you probably have code that resembles the following.
(Though, hopefully, you wouldn't write this actual method)
public int Add(int xInt, int yInt)
{ return xInt + yInt;
}
CodeIt.Right does not like the name "int" in the parameters and this reflects a .NET
Framework guideline.
Here, we find something a single language developer may not stop to consider.
Specifically, not all languages that target the .NET framework use the same type name
conveniences. You say "int" and a VB developer says "Integer." So if a
VB developer invokes your method from a library, she may find this confusing.
That said, I would like to take this one step further and advise that you avoid baking
types into your parameter/variable names in general. Want to know why?
Let's consider a likely outcome of some project manager coming along and saying, "we
want to expand the add method to be able to handle really big numbers." Oh,
well, simple enough!
public long Add(long xInt, long yInt)
{ return xInt + yInt;
}
You just needed to change the datatypes to long, and voilà! Everything went
perfectly until someone asked you at code review why you have a long called "xInt."
Oops. You totally didn't even think about the variable names.
You'll be more careful next time. Well, I'd advise avoiding "next time" completely
by getting out of this naming habit. The IDE can tell you the type of a variable
-- don't encode it into the name redundantly.
Until Next Time
As I said in the introductory part of the post, I believe huge value exists in understanding
code analysis rules. You make better decisions, have better conversations, and
get more mileage out of the tooling. In general, this understanding makes you
a better developer. So I plan to continue with these explanatory posts from
time to time. Stay tuned!
Learn
more how CodeIt.Right can help you automate code reviews and improve your code quality.
About the Author
Erik Dietrich
I'm a passionate software developer and active blogger. Read about me at my
site. View
all posts by Erik Dietrich
|
-
Today we released CodeIt.Right
v2.2 - new version that adds introduces 23 new rules – Usage and Asynchronous
programming best practices – as well as new feature allowing to output analysis results
to Visual Studio Error List in addition or instead of CodeIt.Right Violations Report.
Here is high level list of new features:
-
Added 12 new Async Best Practice Rules
-
Call Start on the Task object before instantiating
-
Async method should have "Async" suffix
-
Async method should return Task or Task<t>
-
Avoid "out" and "ref" parameters in async method
-
TAP method parameters should be the same
-
Do not create async Sub method
-
Transform simple async method to non-async
-
Async method should have await statement
-
Await statement method should be async
-
Do not use Task.Yield in async method
-
Do not use Task.Wait in async method
-
Added 11 new Usage Rules
-
Avoid empty methods
-
Avoid System.Console "Write()" or "WriteLine()"
-
Do not explicitly call "System.GC.Collect()" or "System.GC.Collect(int)"
-
Lock both when either set or get is locked for a property
-
Close database connections in "finally" block
-
Avoid control statements with empty bodies
-
Provide "default:" for each "switch" statement
-
Always provide names for threads
-
Avoid use of "new" keyword for hiding methods
-
Always close SQL resources
-
New Show violations in Error List feature- now OnDemand analysis results within Visual
Studio can be rendered into Violations Report, VS Error List or both
-
and more
CodeIt.Right v2.2 has many more features and improvements. For detailed list please
see What’s
New in CodeIt.Right v2.2
How do I try it?
Download the CodeIt.Right v2.2 at http://submain.com/download/codeit.right/
|
-
We are excited to announce the release of CodeIt.Right
v2.0 - new major version that takes our code quality product to the whole
new level. This version introduces new major feature – Instant Code Review – that
enables developers to get code quality feedback in real time as they type and refactor
code smells right at their introduction. We also added new Personal Edition of the
product. Here is the high level new feature list:
-
 Instant
Code Review feature – get code quality feedback as you code and refactor
on the spot!
-
Visual Studio 11 support
-
Multiple categories for a rule
-
Multi-select in Violations Report
-
XAML Parser
-
8 new Silverlight/WPF/XAML rules
-
27 new ASP.NET/Security rules
-
Profile Editor - filter for selected/unselected rules
-
Option to require comments when excluding code issue
-
and more
Focus on coding – we will help you with quality
If you love the CodeIt.Right code quality rules and auto-corrections but want immediate
feedback as you code, the new Instant Review feature is for you!
Instant Review allows to run select set of rules in the background and get real-time
code issues feedback to developers in the Visual Studio Editor. The feature highlights
in the editor code elements that triggered violation and shows complete list of file
code issues in the right violations bar (next to the scrollbar). Violation detail
window explains the nature of the issue, offers auto-refactoring options and option
to ignore (exclude) the violation. The feature can be turned on/off with a single
click, supports multiple user configurable profiles that can be switched in the toolbar
or right margin violations bar context menu.
 Personal
Edition
Starting version 2.0 in addition to Standard and Enterprise editions we are offering
new Personal Edition designed for solo developers and freelancers. This edition
is priced appropriately for personal use – this is the most affordable edition of
CodeIt.Right. With introduction of the new edition, price of the Standard has been
adjusted.
Is that it?
CodeIt.Right v2.0 has many more features and improvements. For detailed list please
see What’s
New in CodeIt.Right v2.0
How do I try it?
Download the CodeIt.Right v2.0 at http://submain.com/download/codeit.right/
Note to current users – we have changed licensing schema in v2.0
and your v1.x license codes won’t work with v2.0. For users whose Software Assurance
is up-to-date we will be sending v2.x license codes shortly. Users without subscription
and those whose subscription lapsed will have the opportunity to purchase new version
at the upgrade price.
Note to current Standard Edition users – in version 2.0 we have added
"Standard" edition name to all folder locations (Program Files, My Documents, etc)
and registry keys. When you install v2.0 Beta you will need to copy your custom profiles
and rules into the new folders.
|
-
The BETA for CodeIt.Right has arrived and this is our new major version
of the code quality product:
-
Instant
Code Review feature – get code quality feedback as you code and refactor
on the spot!
-
Visual Studio 11 support
-
Multiple categories for a rule
-
Multi-select in Violations Report
-
XAML Parser
-
8 new Silverlight/WPF/XAML rules
-
27 new ASP.NET/Security rules
-
Profile Editor - filter for selected/unselected rules
-
Option to require exclude comments
-
and more
Focus on coding – we will help you with quality
If you love the CodeIt.Right code quality rules and auto-corrections but want an immediate
feedback as you code, the new Instant Code Review feature is for
you!
Instant Review allows to run select set of rules in the background and get real-time
code issues feedback to developers in the Visual Studio Editor. The feature highlights
code elements that raised violation in the editor and shows complete list of file
code issues in the right violations bar (next to the scrollbar). Violation detail
window explains the nature of the issue, offers auto-refactoring options and option
to ignore (exclude) the violation. The feature can be turned on/off with a single
click, supports multiple user configurable profiles that can be switched in the toolbar
or right margin violations bar context menu.
Is
that it?
CodeIt.Right v2.0 has many more features and improvements. For detailed list please
see What’s
New in CodeIt.Right v2.0
How do I try it?
Download the BETA at http://submain.com/download/codeit.right/beta
Where do I post feedback?
Please post your v2.0 Beta feedback in the CodeIt.Right
v2.0 Beta forum
Note to current Standard Edition users – in version 2.0 we have added
"Standard" edition name to all folder locations (Program Files, My Documents, etc)
and registry keys. When you install v2.0 Beta you will need to copy your custom profiles
and rules into the new folders.
Note to all Beta users: Even though CodeIt.Right v2.0 Beta is stable
it's Beta nevertheless. Proceed with care.
|
-
We are added 2 new community contributions to our Tutorials page.
Paulo Morgado posted a great template for CodeIt.Right Code File Header correction
action that automatically generates file header compliant with StyleCop Rules. You
can find the template source in Paulo's blog post CodeIt.Right
Code File Header Template For StyleCop Rules
Craig Sutherland has done a great job integrating
CodeIt.Right with CruiseControl.NET.
Here is CodeIt.Right violations report in CC.NET - great to see Craig took advantage
of the Severity Threshold feature and implemented filtering to reduce "noise" in the
report very much like we have it in CodeIt.Right:
CC.NET CodeIt.Right Analysis Summary report screenshot:
Thank you, Paulo and Craig! Great work!
|
-
Today we released update for CodeIt.Right Standard and Enterprise Editions -
v1.9.10053. As part of our 2010
Product Roadmap, this version changes include significantly improved performance,
support for GlobalSuppressions, new rules and bug fixes.
New in CodeIt.Right v1.9.10053:
-
Major performance improvements throughout the rule base
-
Added support for GlobalSuppressions - syntax is the same
as the VSTS GlobalSuppressions file. This new feature is supported in both Standard
and Enterprise editions.
-
SuppressMessage attribute for class now applied to all member of the class (see example
at the bottom of the post)
-
New Rules:
-
Avoid the Page.DataBind method (AspNet)
-
Avoid the DataBinder.Eval method (AspNet)
-
Do not use SaveAs method to store uploaded files (AspNet)
-
Always define a global error handler (AspNet)
-
Do not disable custom errors (AspNet)
-
Avoid setting the AutoPostBack property to True (AspNet)
-
Interface methods should be callable by child types (Design)
-
Remove unused parameters (Usage)
-
Corrected download redirect link in the Enterprise Edition when new version is available.
In the past versions it would incorrectly open the Standard Edition download page
-
Fixed Encapsulate Field refactoring issue
-
Other fixes for the reported issues
The following code sample on how class level SuppressMessage attribute excludes TypesAndMembersShouldHaveXmlComments
rule for the class and all its members (bullet #3 above):
[SuppressMessage("SubMain.CodeItRight.Rules.General",
"GE00005:TypesAndMembersShouldHaveXmlComments")]
public class MyUndocumentedClass
{
public void MyUndocumentedMethod1{}
public void MyUndocumentedMethod2{}
}
This update is free for all users who are current on their Software
Assurance and Support Subscription
|
-
Code Quality developer tools is the direction we've been following since the introduction
of CodeIt.Right and we are taking this commitment to the next level in 2010 with two
new products and new features for our existing products. One of the new products
to be released in 2010 will assist in unit testing, code coverage and test code profiling;
the second new product will be complementary to CodeIt.Right. All three products
together will comprise our new Code Quality Suite. Additionally, we will continue
to keep up with the Visual Studio 2010 release schedule and have all of our products
2010 compatible when VS2010 is RTM.
Here is what we are planning for 2010:
-
New product!
-
Coming March 2010: we are adding to our product line by offering a unit test
runner and code coverage product.
-
New product!
-
Project Anelare (code name) - we will provide details on this project as we
get closer to a public preview. At this point we can share that this will be
product complementary to CodeIt.Right - together they will encompass our code quality
package.
-
VS2010 support
-
For all products - most of our products are compatible with VS2010 RC, and
we will be VS2010 RTM compatible by the time it RTMs.
-
CodeIt.Right
-
Optimized rule library performance: the new version will be released
the first week in March!
-
Community Rule Valuation & Review: we are pioneering "social" in code analysis
by enabling the community to rate rules and provide feedback; as well as leverage
the community feedback, best uses and best practices for each rule.
-
NEW Rules - with emphasis on security, FxCop/StyleCop parity, SharePoint, WPF
& Silverlight rules.
-
(EE) Trend Analysis: monitor code quality improvements over time.
-
(EE) Integration with manual code review tools.
-
Global Suppressions: adding support for GlobalSuppressions and extending
syntax of the SuppressMessage attribute for more flexible in-code exclusions.
-
Multi-select in the violations list.
-
Copy Rule feature: clone and change rule instance configuration
-
Command line enhancements: open command line/build violations output in Visual
Studio for correction
-
Annotation: for excludes and corrections
-
XAML support: enables building Silverlight and WPF specific rules
-
Profile Wizard: quick start no-brainer user/project profile based on
the project type, importance, community valuation, favorite food, etc
-
GhostDoc
-
We are currently prioritizing the feature set for the new version of GhostDoc. If
you have a feature request that you have not submitted yet, share them with us in
the GhostDoc
forum.
Stay tuned to our blog for more details about our progress!
|
-
New version of CodeIt.Right Enterprise
Edition - build 1.9.09355 - is available for download now.
This version introduces new feature Merge Profiles that allows
to compare and merge user configured profiles, enhances VSTS integration with the
new "Add WorkItem" feature, adds command line version parameters and ability to load/unload
CodeIt.Right in the menu and Add-In Manager.
This
update is free for all users who are current on their Software
Assurance and Gold Support Subscription
New in CodeIt.Right Enterprise v1.9:
-
New Merge Profiles feature allows to compare and merge user configured
profiles
-
New "Add WorkItem" feature - create TFS WorkItem from a violation
-
Added /metrics parameter to the command line utility to generate
XML output for three metrics reports – "Member", "Type" and "Code"
-
Added /sendto parameter to the command line utility - send the violation/metrics
output via email
-
CodeIt.Right can now be loaded/unloaded in the menu and Add-In Manager
-
Build server setup doesn't require Visual Studio on the build machine anymore
-
"Built-in profile" option now is not selectable in the Analysis Module when one or
more custom profiles deployed via Team Configuration Module
-
New Rules:
-
Specify CultureInfo (Globalization)
-
Specify IFormatProvider (Globalization)
-
Specify StringComparison (Globalization)
-
Avoid excessive complexity (Maintainability)
-
Avoid excessive inheritance (Maintainability)
-
Do not use deprecated properties of Response object (AspNet)
-
Fixed a number of bugs in the application and the rules...
|
-
Today we are releasing new version of CodeIt.Right -
build 1.8.09300.
This version features complete ASP.NET support, it is compatible with VS2010
Beta 2 (in addition to VS2008, VS2005, VS2003), Add Rule dialog improvements
- "Hide Rules in the Profile" and "Quick Search", 10+ new rules including new category
"CodingStyle".
Those of you who are using CodeIt.Right with very large solutions may know of
the memory limitation issue for the tools that live in the Visual Studio address space,
aka "Out of Memory" exceptions. You will be excited to know we have addressed the
issue in this version by introducing new "Memory Optimization" mode.
The "Performance mode" is still on by default since it offers a little faster analysis
but CodeIt.Right will offer to switch to the "Memory Optimization" option when
you open a large solution or get the dreaded "Out of Memory" exception. You also are
free to switch between the options manually.
Another change we made - we removed Sealed modifier for all built-in rules.
This gives you even easier way to extend and customize existing rules by simply overriding
just the methods you wanted changed. Over the next couple of weeks we
will be adding a tutorial on how to do that. Ping
us if you want this sooner.
This
update is free for all users who are current on their Software
Assurance and Gold Support Subscription
New in CodeIt.Right v1.8:
-
Now compatible with VS2010 Beta 2, VS2008, VS2005 and VS2003
-
Finally complete ASP.NET support including refactorings in HTML markup.
-
Added Memory Optimization mode – allows to minimize the
memory use at slightly lower reduces performance. This allows to solve the “Out of
Memory” issue on large solutions.
-
Added Performance tab to the Options window – Best Performance/Memory Optimization.
-
Added new Exclude tab and moved all exclude tabs there
-
All Exclude tabs now support multi-select
-
New option to Exclude a Project
-
Added new “Add” button in Exclude Rule, Exclude File and Exclude Project tabs – the
dialogs support multi-select.
-
Profile Editor -> Add Rule dialog has new option Hide Rules in the Profile which
removes from the selection the rules that already exist in the selected profile.
-
Profile Editor -> Add Rule dialog has new Quick Search that filters the
list for the rules that contain entered substring
-
Added RuleID to rule help documentation.
-
Product license codes when entered are now activated on the SubMain server.
-
Auto Update wizard now shows Software Assurance & Gold Support subscription expiration
date.
-
About window now shows Software Assurance & Gold Support subscription expiration
date.
-
All web services – Auto Update, Error Reporting, Statistics and Activation – are now
accessed over SSL protected HTTPS connection.
-
Removed Sealed modifier for all built-in rules. This allows for new
easier way to extend and customize existing rules by simply overriding specific methods.
(Sample is coming)
-
Addressed issues related to incorrect source file encoding when Byte Order Mark (BOM)
attribute was missing.
-
New Rules:
-
Abstract class should have at least one derive class (Design)
-
Interface should have at least one implementation (Design)
-
Project should have AssemblyInfo file (Design)
-
Do not place assembly attributes outside of AssemblyInfo file (Design)
-
Do not include multiple statements on a single line (CodingStyle)
-
Avoid single line If statement (CodingStyle)
-
Do not check for empty strings using Equals (Performance)
-
XML Comments should be spelled correctly (Spelling)
-
Avoid non-public fields in ComVisible value types (Interoperability)
-
Avoid static members in ComVisible types (Interoperability)
-
PInvokes should not be visible (Interoperability)
-
Fixed a number of bugs in the application and the rules...
Download
Download version 1.8.09300 here - http://submain.com/download/codeit.right
Technorati Tags: CodeIt.Right, Coding
Guidelines, Naming
Conventions, Best
Practices, .NET, NET
|
-
As announced
earlier today we just closed the acquisition of the popular XML Comment helper
tool GhostDoc. We, SubMain, will continue to evolve the tool and distribute it as
a free product.
We are also announcing today the availability of the new version of GhostDoc v2.5.09150
which improves the user setup experience, adds support for Visual Studio 2010 Beta
1 and full support for Visual Basic.
Additionally, today we are making available a new version of CodeIt.Right (v1.6.09151)
that adds the IntelliComment feature based on the GhostDoc algorithm and offers improved
and automated generation of XML Comments.
We are very excited about taking over the future of an excellent tool such as GhostDoc!
We are committed to maintaining this wonderful free tool and we welcome the community feedback
and suggestions.
For more on the agreement, please see the press
release and interview with Serge Baranovsky and Roland Weigelt - What's
in the shop for GhostDoc?
New in GhostDoc v2.5:
-
Compatible with VS2010
-
Support for VB - GhostDoc now has full support for VB
-
Removed "Enable experimental support for VB" option in Settings.
-
Improved product setup experience
-
Single setup for all supported versions of Visual Studio - VS2005, VS2008 and VS2010.
-
Setup will detect older version installed and automatically uninstall it.
-
Converted from VS Add-In to VS Package
-
Resolved installation issues related to the VS Add-In model - by converting to VS
Package
Download
Download GhostDoc v2.5.09150 here - http://submain.com/download/ghostdoc
Download CodeIt.Right v1.6.09151 here - http://submain.com/download/codeit.right
|
|
|
|
|
|
|