tags:

views:

328

answers:

9

As a young(ish) developer in a solo shop I have come to greatly appreciate Resharper as a learning tool and mini-QA. That being said, can Resharper break your code in some cases(possibly edge)?

For example(see below), R# continually suggests that my -->

this. is redundant and catch is redundant

There are other examples as well but I am not asking about a specific instance as much as in general.

Has anyone had a R# suggestion break their code? If so what were they and mabye most importantly for me; What can I look for in order to understand if it is, in fact, giving me bad advice?

 private void LoadMyForm(DataRow row)
    {

        try
        {
            this.tbxFirstName.Text = row["FirstName"].ToString();
            this.tbxLastName.Text = row["LastName"].ToString();
            tbxMiddleName.Text = row["MiddleName"].ToString();
            tbxPersonID.Text = row["PersonID"].ToString();
            tbxSSN.Text = row["SSN"].ToString();
            tbxEmailAddress.Text = row["EmailAddress"].ToString();
        }
        catch (Exception)
        {
            throw;
        }
    }
+3  A: 

Sure! You could use it to refactor and break your code. You could use it to rename something through your solution and have it break your code. The key here is that resharper is not really going to break your code...but your use of it inappropriately certainly could! I have never had any issues with any of the suggestions breaking my code. You can turn off almost all of the features of resharper via the options panel!

Andrew Siemer
Really? It's broken our code numerous times. More often with the quick-fixes than the refactorings, though.
Joe White
+10  A: 

Catching your exception only to throw it certainly is redundant, same with the 'this' in this case.

The catch would cease to be redundant if you actually did something when you caught it, rather than rethrowing it.

Chad Ruppert
Also, catching all Exceptions is a _bad_ idea, except in very limited cases.
Talljoe
Good point, very true. Catch specific, expected exceptions and handle those. Anything else SHOULD not be caught.
Chad Ruppert
The only case I can see when it would be justified to carch all Exceptions is when writing your own error handler (to log uncaught exceptions and such), but then again, you are better off attaching an handler to AppDomain.CurrentDomain.UnhandledException rather than a "try {} catch(Exception e)".
Andrew Moore
And to add to my comment above, you only want to attach such handler when no debugger is attached. (!System.Diagnostics.Debugger.IsAttached)
Andrew Moore
+1  A: 

The suggestions you got are the default settings suggested by ReSharper. Howerver, I find a lot of suggestions are not consistent with my normal way of coding style, so I disable it. You can easily disable it with the ReSharper suggestion on the same place.

The good thing about refactoring is that sometimes you are not aware that some newly C#3.0 features can simplify your code, and ReSharper is very helpful in this regard ( ask you use Lambda expression instead of anonymous method....).

J.W.
A: 

It sure can break your code but it always warns you first about possible problems. You can bend resharper to your will by disabling its suggestions. I have been using it for years now and I kinda feel relaxed when I see that green rectangle on the top right of my text editor :)

idursun
+3  A: 

ReSharper is not perfect (as is the case with all software). It definitely has bugs and will occasionally offer suggestions that are incorrect and will break your code. However I've found these to be very rare and usually involves some very nested generic scenarios.

In general, if ReSharper is suggesting something it is correct. In this case ReSharper is correct because the catch statement effectively does nothing. A throw without an exception target will rethrow the original vs. throwing a new exception and hence there should be no visible side effects.

JaredPar
A: 

The biggest Problem I have ever had is when I do a refactoring and it doesnt take effect everywhere that it should. This occurs in a large project where I may not have all of the referencing components loaded in a single solution at any given time, or when I am refectoring from a location that is rather distant from the declaration. Just be careful and you'll be alright.

CaptnCraig
+3  A: 

ReSharper is not infallible. It's not the same as someone sitting there with you, reading your code. It does its best. That said, the gains of R# far outweigh the losses, and nobody should blindly follow R#'s suggestions.

R# once suggested a change to my code that was definitely breaking. I had (somewhat pseudocode):

var submitBtn = (Button)Controls.FindControl(blahblahblah);

R# told me to wrap it in a using. Doing so would destroy my button control before I was finished with its scope. Of course, it was less R#'s mistake than my own, but there ya go.

Robert S.
In this case, ReSharper didn't suggest it. You had no mark in the editor. Light bulb shows not only items that are suggested, but also handy editing options. E.g. you can invert if ad infinum :)
Ilya Ryzhenkov
While you're certainly the R# expert, I don't think I'm alone in taking what R# puts in the light bulb as a suggestion.
Robert S.
Also in a sense R# *is* like someone sitting there with you reading your code - any such real person would also not be infallible, after all. I think the point is that for *most* developers, R# will be like someone less fallible than the developer themselves :)
AakashM
+4  A: 

In my opinion Resharper needs to be viewed as an extension of your own personal knowledge of C# and software development principles in general. If you look at a suggested R# refactoring and don't understand the code that it suggests changing it to... DON'T DO IT.

Resharper is probably (in my experience) right, but if it is going to be changing your code into something that you don't understand and can't easily read then its not doing you any favors.

I would recommend researching the code and the concept that it suggests using and make an effort to understand what the tool is advising you to do. Only then can you make the judgement call on whether or not a given suggestion works in YOUR situation.

If you continue like this and use the tool to further your knowledge then eventually R# will start to serve as a gentle reminder of best practices and coding techniques as you write your software.

Ideally the more you use the tool the less you should be relying on it as you should start incorporating the concepts into your first pass at the code and should stop needing the suggestions it gives you in the first place.

Max Schilling
+1  A: 

The only way you will come to learn what suggestions you should and shouldn't heed is through experience.

Personally as a coding style I like to refer to all member variables as this.something as it makes it easier to identify member variables vs local fields when glancing over code, other people might disagree for different reasons.

Also, catching and re throwing the same exception is not only redundant but actually can be a pretty bad idea as catching and re throwing is not the same as not catching in the first place, and can break your call stack. See http://weblogs.asp.net/fmarguerie/archive/2008/01/02/rethrowing-exceptions-and-preserving-the-full-call-stack-trace.aspx

Resharper sounds like a nice tool for pointing possibilities out, but you shouldn't take every suggestion for gospel. I find that every now and then something happens which makes me realise why a coding practice that I'm using is a bad idea - like loosing a stack trace for example. Ultimately however I wouldn't worry too much as things like that can only come from experience.

Kragen