50 Versions of Shed Control Wars
-
So i've been chasing an "EntityFramework bug" for the past couple of days in some code i inherited when a coworker got promoted to management (different one than i was celebrating yesterday). this code had been newly refactored but not tested.
I eventually found the issue and it was that ReSharper had indicated a code refactor that my coworker had accepted.
Resharper indicated that the following code…
get { if (this.context == null) this.context = new OurAwesomeDataContext(); return this.context; }
… could be refactored to…
get { return this.context ?? new OurAwesomeDataContext(); }
umm..... i don't think that is refactor without side effects.... can you see why?
-
It probably got confused by the lack of braces, AKA TRWTF.
-
It probably got confused by the lack of braces
well maybe. i'll certainly be adding a lot of those to this project!
-
i don't think that is refactor without side effects.... can you see why?
Discorefactor?
-
can you see why?
This is just ReSharper's passive-aggressive way of saying that C# needs a ??= operator.
-
You need to upgrade Resharper.
private string context; public string Foo { get { if (this.context == null) { this.context = "foo"; } return this.context; } }
Gets turned into this using R# 8.2:
public string Foo { get { return this.context ?? (this.context = "foo"); } }
Which is correct, because assignment in C# returns the assigned value. In fact, I use this pattern all the time for lazy initialization.
-
You need to upgrade Resharper.
hmm
Resharper version:
and i did just reproduce it. without the braces as originally posted it corrects to the wrong version. with the braces it corrects to the working version.
-
without the braces as originally posted it corrects to the wrong version. with the braces it corrects to the working version.
#WINNING
-
want a cupcake?
-
No thanks. Trying to cut back after the holidays.
-
Not even if they have fruit?
On a related note:
WANT
-
JetBrains products are always confident they can refactor your code for you. I never let them mess with my javascript stuff, but I thought statically typed code is safe enough. I guess not.
-
i opened a bug report. i'm sure they'll fix it in the next update.
-
Hey, I'm just trying to help... an isolated example doesn't reproduce the issue.
I have VS2013.4, R#8.2.3. I'd post screenshots, but assholes on the internet force this site to not let new users post images. Braces or no, it corrects to what I posted. I even modified the code to make it use an object constructor in the assignment.
private Tuple<int, int> context;
public Tuple<int, int> Foo { get { if (this.context == null) this.context = new Tuple<int, int>(1, 2); return this.context; } }
and it's turned into this:
private Tuple<int, int> context; public Tuple<int, int> Foo { get { return this.context ?? (this.context = new Tuple<int, int>(1, 2)); } }
I don't see how it produced this wrong code, and I've never seen it do something so semantically wrong.
i opened a bug report.
Care to link? I'd like to follow this rabbit hole.
What's also funny is that he followed the recommendation for "Use ??" but not "Remove Redundant Qualifier".
/no I do not work for JetBrains
-
I have VS2013.4, R#8.2.3. I'd post screenshots, but assholes on the internet force this site to not let new users post images. Braces or no, it corrects to what I posted. I even modified the code to make it use an object constructor in the assignment.
huh. maybe it's somthing to do with entity framework then? the context was creating an entity framework data context
failing that possibly a user setting we tweaked?
i opened a bug report.
on mobile at the moment. will post when i get back to the office tomorrow
-
Not even if they have fruit?
In general, I don't eat fruit. The cookie monster one looks good, though.
-
GAH my eyes!!! What IS that!?
-
What IS that!?
Nothing. I once maintained an app that had an Escher print as the splash screen.
-
Poor man's double-lock-checking?
Lock-checking-for-poor-men?
All-of-the-above?
-
GAH my eyes!!! What IS that!?
resharper's about screen.
first time i've seen it actually.
-
Am I the only one who thinks the resharpered version (the corrected one) is needlessly confusing and that doing assignment on the RHS of a coalesce is a horrible thing to do?
It probably got confused by the lack of braces, AKA TRWTF.
QFT
-
Am I the only one who thinks the resharpered version (the corrected one) is needlessly confusing and that doing assignment on the RHS of a coalesce is a horrible thing to do?
...but the resultant code is shorter, so it's less likely to have bugs in it, statistically speaking?
-
yeah, and the more time you spend driving the more likely you are to crash, so go everywhere at 200mph
-
Finally, someone who gets my driving philosophy
-
-
...but the resultant code is shorter, so it's less likely to have bugs in it, statistically speaking?
Are you C developer?
-
...but the resultant code is shorter, so it's less likely to have bugs in it, statistically speaking?
Yes, but no one really understands what statistics mean, kinda like terse, operator heavy code is easy to read quickly and miss important details about.
-
Hey, I'm just trying to help... an isolated example doesn't reproduce the issue.
I have VS2013.4, R#8.2.3. I'd post screenshots, but assholes on the internet force this site to not let new users post images. Braces or no, it corrects to what I posted. I even modified the code to make it use an object constructor in the assignment.
found the issue BTW.
while i am currently running on 8.2.3 on my desktop, my coworker isn't. he's back on 8.1 something. and the resharper local projects settings file/folder thingie was checked into source, so we've accidentally merged both of our preferred settings (mostly how many warnings to show. i like to see them all and decide to ignore individual warnings with code comments if i decline the refactor, he just wants to write code.)
anyway regenerating a clean 8.2.3 set of settings makes the problem go away. so it was either some weirdness when the different versions got merged by SVN or it was the resultant combination of our settings that did it.
either way i made it stop generating bad suggestions.
-
TRWTF is not setting proper ignores in repo.
-
TRWTF is not setting proper ignores in repo.
TRRWTF is SVN not having a good model for setting ignore patterns.
seriously look at it it's terrible!
i'm currently lobying for a switch to GIT...or back to TFS, WTF was wrong with TFS that we had to switch? i mean i know it's tied to visual studio and all but 99.9999999999999999% of the code in the repo is C# and the rest is mostly legacy VB
I might prefer GIT personally over TFS, but TFS was working!
-sigh-
-
Wait a minute... I just realized that for the config file to be a problem, someone had to commit it first. This is TRRRWTF
-
Wait a minute... I just realized that for the config file to be a problem, someone had to commit it first. This is TRRRWTF
SVN blame says it was coworker and i didn't notice. :-P
-
TRRWTF is SVN not having a good model for setting ignore patterns.
seriously look at it it's terrible!
Really? I've never had any problems getting my svn:ignore properties set up? Was I just lucky? (I was probably using TortoiseSVN when I did it, so...)
-
WTF was wrong with TFS
We moved from Perforce to TFS. I still [i]to this day[/i] miss changelists. :<sxds>(
-
I've never had any problems getting my svn:ignore properties set up?
those are hell to maintain though and putting some on child directories completley override the ones from parent dirs which leads to lots of duplication.
the GIT model is much better. just one .gitignore and done.
-
-
Ok, I see. Why would you need to do that?
because some projects have different needs?
\and no adding .svnignore isn't an option.... for whatever reason it's not supported by the SVN server we've setup.
-
some projects have different needs?
Ah, I guess this is the part where I say "Well, there's your real problem!"
-
-
"Weeeeaaalll, thaaar's yarr reaall praablam!"
-
that's better...... but could you add a little cowbell? everyhtings better with cowbell!
-
-
found the issue BTW.
[snip versioning and config explanation]
Glad you found the problem!
I've never run into a team that all used Resharper... it's mostly a personal thing.
-
I've never run into a team that all used Resharper... it's mostly a personal thing.
my first time too. i'm glad we use it, but dang that was a weird bug!
-
i like to see them all and decide to ignore individual warnings with code comments if i decline the refactor, he just wants to write code.
/me approves of this.
That's exactly how I work (on my projects) with IntelliJ
Of course, the project at work has so many warnings that doing that isn't feasible, but I can at least try...
-
Am I the only one who thinks the resharpered version (the corrected one) is needlessly confusing and that doing assignment on the RHS of a coalesce is a horrible thing to do?
All terse idioms look needlessly confusing until you are used to them, and to be honest, that's the only time I ever use that idiom: lazy initialization of a private field and return of the value. It's probably the only "weird" C# idiom I've seen in common use, the other being using "_" as the dummy variable in LINQ queries.
-
Seems like this is the current place for Git fans - can you checkout a file in the traditional sense that it stops other people touching it until it's checked back in?
I did about 3 seconds of Google-fu then figured someone here would know.
-
Seems like this is the current place for Git fans - can you checkout a file in the traditional sense that it stops other people touching it until it's checked back in?
No you can't. You also can't force commits to be "online", without requiring a manual push/sync each time.
Those were some of my early complaints when I first had to use Git. I'll never figure out in a million years why they thought it was a good idea to get rid of "online" mode. (Sure, being able to work offline is a nice feature-- but why FORCE users to work offline?)
-
Seems like this is the current place for Git fans - can you checkout a file in the traditional sense that it stops other people touching it until it's checked back in?
survey says no.
of course i would argue that the idea of exclusively locking a file is a dangerous on in the first place and should be avoided if possible
what use case are you thinking of that desires file locking?
-
what use case are you thinking of that desires file locking?
"I'm editing this motherfucker and you can't until I'm done"