50 Versions of Shed Control Wars


  • sockdevs

    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.


  • sockdevs

    @boomzilla said:

    It probably got confused by the lack of braces

    well maybe. i'll certainly be adding a lot of those to this project!


  • sockdevs

    @accalia said:

    i don't think that is refactor without side effects.... can you see why?

    Discorefactor?



  • @accalia said:

    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.


  • sockdevs

    @gestahl said:

    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.



  • @accalia said:

    without the braces as originally posted it corrects to the wrong version. with the braces it corrects to the working version.

    #WINNING


  • sockdevs

    want a cupcake?



  • No thanks. Trying to cut back after the holidays.


  • sockdevs

    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.


  • sockdevs

    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


  • sockdevs

    @gestahl said:

    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?

    @gestahl said:

    i opened a bug report.

    on mobile at the moment. will post when i get back to the office tomorrow



  • @RaceProUK said:

    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!?


  • Discourse touched me in a no-no place

    @blakeyrat said:

    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?


  • sockdevs

    @blakeyrat said:

    GAH my eyes!!! What IS that!?

    resharper's about screen.

    first time i've seen it actually.


  • BINNED

    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?

    @boomzilla said:

    It probably got confused by the lack of braces, AKA TRWTF.

    QFT



  • @Jaloopa said:

    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?


  • BINNED

    yeah, and the more time you spend driving the more likely you are to crash, so go everywhere at 200mph


  • Notification Spam Recipient

    Finally, someone who gets my driving philosophy


  • Discourse touched me in a no-no place



  • @tar said:

    ...but the resultant code is shorter, so it's less likely to have bugs in it, statistically speaking?

    Are you C developer?



  • @tar said:

    ...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.


  • sockdevs

    @gestahl said:

    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.


  • sockdevs

    @Gaska said:

    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


  • sockdevs

    @Gaska said:

    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



  • @accalia said:

    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...)



  • @accalia said:

    WTF was wrong with TFS

    We moved from Perforce to TFS. I still [i]to this day[/i] miss changelists. :<sxds>(


  • sockdevs

    @tar said:

    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.



  • @accalia said:

    putting some on child directories

    Ok, I see. Why would you need to do that?


  • sockdevs

    @tar said:

    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.



  • @accalia said:

    some projects have different needs?

    Ah, I guess this is the part where I say "Well, there's your real problem!"


  • sockdevs

    @tar said:

    "Well, there's your real problem!"

    needs more hillbilly accent.

    :-P



  • "Weeeeaaalll, thaaar's yarr reaall praablam!"


  • sockdevs

    that's better...... but could you add a little cowbell? everyhtings better with cowbell!



  • :heavy_plus_sign: :cow2: :bell:



  • @accalia said:

    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.


  • sockdevs

    @gestahl said:

    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!


  • Winner of the 2016 Presidential Election

    @accalia said:

    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...



  • @Jaloopa said:

    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.



  • @loopback0 said:

    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?)


  • sockdevs

    @loopback0 said:

    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?



  • @accalia said:

    what use case are you thinking of that desires file locking?

    "I'm editing this motherfucker and you can't until I'm done"


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.