Tech Off Thread

41 posts

Forum Read Only

This forum has been made read only by the site admins. No new threads or comments can be added.

is 'new object().property' or 'new object().method()' a good idea?

Back to Forum: Tech Off
  • User profile image
    vbrunner__

    ScanIAm wrote:
    Console.WriteLine(new String("Bob").Length);


    The part that concerns me is the accessing of the function 'Length' when you don't really know that the new String has actually been created.

    Of course you know that the new String has been created.  why would you not?  that's exactly what you are telling the compiler to do. consider:

    namespace ConsoleApplication1
    {
        class Program
        {
            static void Main(string[] args)
            {
                System.Console.WriteLine(new System.String('c', 10).Length);
            }
        }
    }



     here you are telling the runtime to create a new object of type String by calling the constructor with provided parameters, get the Length property, and pass a copy of it to the Console.WriteLine function.  from there it doesn't matter what happens to the String object because it is no longer needed; the WriteLine function has it's own copy (because it's a value type) of what we wanted, so no worries.

    am i right, guys?

  • User profile image
    Richard.Hein

    ScanIAm wrote:
    
    Exactly, and I'm fine with that, now, but it doesn't 'feel' right.  That's my own cross to bear Smiley

    string s = "bob";
    if(s!=null)
       Console.WriteLine(bob.Length);

    just seems safer.




    Sorry, that's just not making sense; maybe you need to come up with a better example, but I can't see it.  s can never be null here ... even if it was you'd sure as hell WANT an exception to be thrown, because it's exceptional ... you don't want to hide this kind of exception.

  • User profile image
    ScanIAm

    Richard.Hein wrote:
    
    ScanIAm wrote:
    
    Exactly, and I'm fine with that, now, but it doesn't 'feel' right.  That's my own cross to bear Smiley

    string s = "bob";
    if(s!=null)
       Console.WriteLine(bob.Length);

    just seems safer.




    Sorry, that's just not making sense; maybe you need to come up with a better example, but I can't see it.  s can never be null here ... even if it was you'd sure as hell WANT an exception to be thrown, because it's exceptional ... you don't want to hide this kind of exception.


    OK, it appears that some of you never started out coding in ASM, C or C++ because you aren't able to envision a time and place where

    1) dinosaurs ruled the planet
    2) memory allocations were managed by the developer
    3) some of them could fail for more reasons than just 'out of memory'.

    So,

    Console.WriteLine(new Object().ToString());

    Would be a great way to cause your application to fail catastrophically.

    It's also evident that from your point of view, failure is simply something that the end user can recover from.  This is not the case when working on server software.  You don't want your application to fail, you want it to retry.

    So, All I'm saying is that I've spent at least 1 decade forcing myself to remember these limitations and when I see

    Console.WriteLine(new Object().ToString());

    It feels 'wrong' to me because, in the past, it WAS wrong, based on my experience.

    I'm quite aware that it isn't wrong, and is, in fact, a simpler way to code.

    I really don't have anything more to say about it.

  • User profile image
    littleguru

    DinoViehland wrote:
    
    littleguru wrote:
    

    The MyObject instance won't be collected by the GC if you are accessing any fields that held by that instance... otherwise - as said - it would be a heavy CLR (GC) bug!


    Just to shed a little light on the situation the typical example of this is using IntPtr's and the Dispose pattern.  Such as:

    class MyDisposable : IDisposable {
        private IntPtr _foo;
        public void DoIt() {
            SomePInvokeOperation(_foo);
        }

        ~MyDisposable() {
            Dispose();
        }
        public void Dispose() {
            CloseMyHandle(_foo);
            GC.SuppressFinalization(this);
        }
    }

    Now you do: new MyDisposable().DoIt();  The IntPtr is read and right after that read there are no longer any references to the MyDisposable() instance.  Before SomePInvokeOperation can run the GC kicks in and collects the MyDisposable class.  Next the finalizer thread runs which runs the finalizer which closes the native resource.  Finally your SomePInvokeOperation runs and is operating on a _foo that is no longer valid.

    The worst case scenario here is that you can use this for a handle recycling attack which violates security.  If you're lucky the app is just going to crash.  Anyway, the way to fix this is to put a GC.KeepAlive(this); in DoIt() so that the object survives the lifetime of the call.  But it's really only a problem when you're interacting with native resources which the GC doesn't understand and keep alive its self.  So still not a bug, but a good thing to know about if you're doing any interop programming.


    Let's sum up: I'm sure that the first parameter that is passed in with that method is a this pointer... Why would the GC mean that the instance should be collected, if I try to access a member variable later on? If I don't access it, it is fine because I'll never discover that the GC collected it.

    I'm accessing a private member of the class in the DoIt method. How can the GC collect the instance that is holding that before I access it. I thought the GC is only collecting if the instance is no longer reachable!

    Correct me if I'm wrong!

  • User profile image
    yinxi88

    :O wow ,my god

  • User profile image
    evildictait​or

    littleguru wrote:
    

    I'm accessing a private member of the class in the DoIt method. How can the GC collect the instance that is holding that before I access it. I thought the GC is only collecting if the instance is no longer reachable!

    Correct me if I'm wrong!


    Turns out this is only happens when interacting with PInvoke and unmanaged operations. Within managed code it doesn't happen.

    Read Dino's post.

  • User profile image
    littleguru

    evildictaitor wrote:
    
    littleguru wrote:
    

    I'm accessing a private member of the class in the DoIt method. How can the GC collect the instance that is holding that before I access it. I thought the GC is only collecting if the instance is no longer reachable!

    Correct me if I'm wrong!


    Turns out this is only happens when interacting with PInvoke and unmanaged operations. Within managed code it doesn't happen.

    Read Dino's post.


    I thought that this has to do with PInvoke. But I guess it has to do while the PInvoke code is running not before that! Smiley

  • User profile image
    evildictait​or

    littleguru wrote:
    
    evildictaitor wrote:
    
    littleguru wrote:
    

    I'm accessing a private member of the class in the DoIt method. How can the GC collect the instance that is holding that before I access it. I thought the GC is only collecting if the instance is no longer reachable!

    Correct me if I'm wrong!


    Turns out this is only happens when interacting with PInvoke and unmanaged operations. Within managed code it doesn't happen.

    Read Dino's post.


    I thought that this has to do with PInvoke. But I guess it has to do while the PInvoke code is running not before that! Smiley


    Yes. It reminds me of one of the really hard-to-repro bugs I've ever seen which was in an app designed to "turn off" the Windows key while in certain applications.

    Basically you have to interact with Windows and give them a delegate object which they call whenever a key is pressed, but sadly even delegate objects get garbage collected, at which point the next keystroke kills the application immediately. Took me weeks to find that bug.

  • User profile image
    Curt Nichols

    evildictaitor wrote:
    To protect against it you could do

    MyObject o = new MyObject();
    var result = o.LongRunningMethod();
    GC.KeepAlive(o);

    But in fairness this is a bug that should be fixed by Microsoft rather than coded around, and you should write an bugreport if you find in your code that this is happening.


    You could use KeepAlive, but it's probably better to fix the design of your code. This is definitely not a bug. Smiley

  • User profile image
    Curt Nichols

    littleguru wrote:
    The MyObject instance won't be collected by the GC if you are accessing any fields that held by that instance... otherwise - as said - it would be a heavy CLR (GC) bug!


    This is incorrect, and it's not a bug. Objects referenced by fields of the MyObject instance will be collected only if when are no rooted references to them, not when the MyObject instance is collected. The MyObject instance may be collected even if there are other outstanding references to objects that its fields refer to. You can see for yourself here, code is included.

  • User profile image
    Curt Nichols

    Pop Catalin Sever wrote:
    1) if LongRunningMethod() acceses fields on MyObject instance the "this" parameter is passed on the stack to the method meaning you have a rooted reference to the object instance, the method call practicaly is like this

    newobj MyObject //Returns a reference in the stack
    call
    LongRunningMethod



    What you're not seeing is that the JITter helps the GC know when the reference is no longer needed. In this case the MyObject reference is needed only to invoke the call to LongRunningMethod, but isn't needed afterward. If a GC occurs during LongRunningMethod the instance may be collected if there are no other rooted references to the object.

    See here if you want code to reproduce the behavior.

  • User profile image
    Dr Herbie

    I would see

    new object().SomeMethod()

    as a 'smell' indicating that SomeMethod should be static ...

     

    Herbie

     

  • User profile image
    shuff1203

    Dr Herbie wrote:
    
    I would see

    new object().SomeMethod()

    as a 'smell' indicating that SomeMethod should be static ...

    Herbie



    Can this even be done in VB.NET?

    I'm trying to convert some sample code from c# that says:

    New Application().run(f)

    But that doesnt' work in VB.NET.  Is there an equivalant?

  • User profile image
    JChung2006

    Dim o = New Application()
    o.run(f)
    

  • User profile image
    shuff1203

    JChung2006 wrote:
    
    Dim o = New Application()
    o.run(f)


    My quesiton was is there an equivalent to:

    New Application().run(f)

    I realize I could declare a var and use it.

  • User profile image
    evildictait​or

    shuff1203 wrote:
    
    My quesiton was is there an equivalent to:

    New Application().run(f)

    I realize I could declare a var and use it.



    Try declaring a function and passing that as the arguments:

    foo( New Application().run(f) )

    If that throws an error, then no you can't, otherwise you might need to declare the type of the variable you're assigning first.

  • User profile image
    JChung2006

    GetType(Application).GetConstructor(New System.Type() {}).Invoke(New Object() {}).Run(f)

  • User profile image
    evildictait​or

    JChung2006 wrote:
    
    GetType(Application).GetConstructor(New System.Type() {}).Invoke(New Object() {}).Run(f)


    While your post is technically correct, using reflection just to inline a function is never a good idea.

Conversation locked

This conversation has been locked by the site admins. No new comments can be made.