Make sure it's initialized!



  • Here's a sample from C#; I see the same thing in Java & C++:

    if (devID != null)
    {
    String hashInput = null;
    byte[] hashOutput = null;
    byte[] hashBytes = null;

    hashInput = String.Format("{0}{1}", devID, Acme.Utility.ByteToHex(devID));
    hashBytes = System.Text.Encoding.UTF8.GetBytes(hashInput);
    hashOutput = Hasher.GenerateHash(hashBytes);
    var = System.Convert.ToBase64String(hashOutput);
    }

    This is not on par with clubbing baby seals to death, but why have a separate line of code to declare and initialize a variable, only to unconditionally set it and not check the result a bit later?

    I started coding C in 1984, I understand that many programmers are used to seeing all the variables declared at the top of a method or block and feel uncomfortable doing anything else, but I think they ought to get over it. Silly code bloat is, well, silly.

    What I'd much prefer is something like:


    String hashInput = String.Format("{0}{1}", devID, Acme.Utility.ByteToHex(otherID));


  • Agreed. In fact many tools will flag such elements as warnings [and I love "Warnings as Errors"



  • I see one reason to split them, and it is a good one.  Thing is, it doesn't apply here.

    When using try catch blocks, you want the assignment of the variable within the try, just in case, especially if it is assigned by something other than a simple static value.  You want to declare the variable above the try block.  Here is why:

    When a variable is assigned above the try it is seen within the catch and finally blocks for cleanup, setting to nothing if needed, and calling dispose in case of an error.  If the variable is declared within the try block, then it is out of scope in the catch and finally.  So in cases like this you want to declare on one line above the try, but assign within the try.  This does not apply to simple types such as strings or integers or really anything that doesn't need a dispose.



  • Or, even better, use using so you don't have to worry about calling Dispose.



  • Not good enough if you actually need to do something in the case of an exception with those instances or need to return the instance you are working with.  You will be outside the using block and even the try block so they won't be there in the catch or finally.  When you need to do this, you need to declare outside the try block so you can work with it inside the block.  If you have a finally block you should not be trying to return from within the try, pass the reference back under the finally or outside of the entire exception block itself.



  • <sarcasm>

    What's wrong with this?

    if (devID != null) var = System.Convert.ToBase64String(Hasher.GenerateHash(System.Text.Encoding.UTF8.GetBytes(String.Format("{0}{1}", devID, Acme.Utility.ByteToHex(devID)))));

    Oh yeah, paid per line...

    </sarcasm>



  • Meh. I consider it good practice to declare your variables at the top of the method. Some style checking tools even have rules for it. 1/10



  • @dtech said:


    Meh. I consider it good practice to declare your variables at the top of the method. Some style checking tools even have rules for it. 1/10

    Is the practice intrinsically good, or is it that you are stuck in an unproductive rut of writing cluttered, unclear code?



  • @dtech said:


    Meh. I consider it good practice to declare your variables at the top of the method. Some style checking tools even have rules for it. 1/10

    Still writing Fortran are you? Allow me to step off your lawn.



  • @rstinejr said:

    @dtech said:


    Meh. I consider it good practice to declare your variables at the top of the method. Some style checking tools even have rules for it. 1/10

    Is the practice intrinsically good, or is it that you are stuck in an unproductive rut of writing cluttered, unclear code?

    EVERY style check I use or know of recommends declaring as close as possible to the point of first usage, NEVER at the top of the method. In some languages  declarations at the top may have a negative performance impact. Can you provide ONE concrete example of a modern (within 3-5 years) style checker that recommends declaration at top instead???



  • @TheCPUWizard said:

    @rstinejr said:
    @dtech said:


    Meh. I consider it good practice to declare your variables at the top of the method. Some style checking tools even have rules for it. 1/10

    Is the practice intrinsically good, or is it that you are stuck in an unproductive rut of writing cluttered, unclear code?

    EVERY style check I use or know of recommends declaring as close as possible to the point of first usage, NEVER at the top of the method. In some languages  declarations at the top may have a negative performance impact. Can you provide ONE concrete example of a modern (within 3-5 years) style checker that recommends declaration at top instead???

    Maybe he's using a tool that transforms all of his code into K&R C..



  • I've heard there are languages that redeclare/reinitialize variables when declaring them inside of a loop.  I don't know what those are or how true it is, but it's certainly not true about C#.  Declaring them at the top of the function is bad form and hard to read.  Declaring stuff outside a try/catch is fine if you need it, but that's not the case here, and is also not the case for calling Dispose... you can do that without having to declare it somewhere else.  You can return an instance from inside a using or try/catch.



  • @Sutherlands said:

    I've heard there are languages that redeclare/reinitialize variables when declaring them inside of a loop.  I don't know what those are or how true it is, but it's certainly not true about C#.  Declaring them at the top of the function is bad form and hard to read.  Declaring stuff outside a try/catch is fine if you need it, but that's not the case here, and is also not the case for calling Dispose... you can do that without having to declare it somewhere else.  You can return an instance from inside a using or try/catch.

    ok try this, yes going to be in VB just because:

    Try

    dim rs as new recordset

    catch

    'who cares its an example

    finally

    rs.dispose

     

    When you get to the dispose you will get an object not declared error, this is .Net itself so VB or C# doesn't matter here, but goes to show that you will need to declare the record set before the try in order to reference it in the finally in order to dispose.  Granted this pattern is not always needed, but when it is needed for whatever reason you need to separate your declarartion from your initiation.  As I said before, this doesn't apply here, but is one of the few cases where I see the declaration and instantiation needing to be separated, "using" also doesn't help here.



  • @Sutherlands said:

    I've heard there are languages that redeclare/reinitialize variables when declaring them inside of a loop.

    Do you mean like JavaScript? If you declare a variable within a function, it "hoists" the declaration so that it effectively takes place at the beginning of the function.


    [How is it that in a forum of computer nerds 90% don't know how to use a goddamn A tag? -ShadowMod]


  • ♿ (Parody)

    @Sutherlands said:

    I've heard there are languages that redeclare/reinitialize variables when declaring them inside of a loop.

    Yes, this is called variable shadowing.



  • try

    {

      using(RecordSet rs = new RecordSet)

      {

        //Do stuff

      }

    }

    catch(Exception)

    {

      //Who cares? It's an example.

    }

     

    Hey look, i managed to dispose it without declaring it outside the try.



  • @gramie said:

    @Sutherlands said:

    I've heard there are languages that redeclare/reinitialize variables when declaring them inside of a loop.

    Do you mean like JavaScript? If you declare a variable within a function, it "hoists" the declaration so that it effectively takes place at the beginning of the function. http://www.adequatelygood.com/2010/2/JavaScript-Scoping-and-Hoisting

    @boomzilla said:

    @Sutherlands said:
    I've heard there are languages that redeclare/reinitialize variables when declaring them inside of a loop.
    Yes, this is called variable shadowing.

    No, and no.  I mean if you write something like
    for(int count = 0; count < 100; ++count) {MyBigObject x = new MyBigObject();  x.PerformAction(count);}

    Some languages/compilers will actually create space for x every time through the loop, making it extremely slow compared to declaring "MyBigObject x" outside the loop then creating the new object in the loop (assuming it's not the same object throughout the entire loop anyway).



  • @Kattman -  Declaring a variable "just before the Try statement" is NOT the same thing as "at the beginning of the method". There are many valid use cases for the former (Dispose and using is a Red Herring), but not for the latter (consider that the try statement could be on line 50 of the method!)



  • @Sutherlands said:

    Some languages/compilers will actually create space for x every time through the loop, making it extremely slow compared to declaring "MyBigObject x" outside the loop then creating the new object in the loop (assuming it's not the same object throughout the entire loop anyway).

     Name one language (must be at least semi-mainstream, and less than 10 years since the latest version) that does that. EVERY language I know of (and I do write compilers) will allocate sufficient stack space on entry to the method (function, subroutine) to handle the worst case variable allocation at any point in time.  The same physical memory location will be used for different variables within the same method wit no "create space" operations being performed, provided that the lifetimes of the variables permit it.



  • @boomzilla said:

    @Sutherlands said:
    I've heard there are languages that redeclare/reinitialize variables when declaring them inside of a loop.

    Yes, this is called variable shadowing.

    I call them "goatee variables" because they look like another variable and they're evil.


  • ♿ (Parody)

    @Sutherlands said:

    I mean if you write something like

    for(int count = 0; count < 100; ++count) {MyBigObject x = new MyBigObject();  x.PerformAction(count);}

    Some languages/compilers will actually create space for x every time through the loop, making it extremely slow compared to declaring "MyBigObject x" outside the loop then creating the new object in the loop (assuming it's not the same object throughout the entire loop anyway).

    Well, with the sort of code you wrote, yes, I'd expect a compiler to create a new object each time you told it to. A super smart compiler might be able to convince itself that PerformAction() doesn't change the state of x, and just reuse it, but I wouldn't rely on that sort of thing. The time drag is calling the constructor to allocate and initialize a new instance, not the storage space for x itself. That is presumably something just sitting around on the stack (or equivalent).

    If a language / compiler did this by default, without knowing that PerformAction() didn't change the state of x, then I'd say that language / compiler is broken.


  • ♿ (Parody)

    @morbiuswilters said:

    @boomzilla said:
    @Sutherlands said:
    I've heard there are languages that redeclare/reinitialize variables when declaring them inside of a loop.

    Yes, this is called variable shadowing.

    I call them "goatee variables" because they look like another variable and they're evil.

    I'm kinda sad / surprised that you didn't change the link when you quoted it.



  • @CarnivorousHippie said:

    <sarcasm>

    What's wrong with this?

    if (devID != null) var = System.Convert.ToBase64String(Hasher.GenerateHash(System.Text.Encoding.UTF8.GetBytes(String.Format("{0}{1}", devID, Acme.Utility.ByteToHex(devID)))));

    Oh yeah, paid per line...

    </sarcasm>

     

    I like yar style, young master. What's wrong with it? All those silly name spaces, that's what. That could've been a perfectly readable

    if (devID != null) var = ToBase64String(GenerateHash(GetBytes(Format("{0}{1}", devID, ByteToHex(devID)))));

    By the way, did anybody wonder what this code does? Concatenate devID and it's "hex" version, hash it, and convert that to base64. Isn't that a bit overkill for a single byte id?



  • @boomzilla said:

    The time drag is calling the constructor to allocate and initialize a new instance, not the storage space for x itself. That is presumably something just sitting around on the stack (or equivalent).

    Does C# store objects on the stack under certain conditions? Because, last time I checked, Java always stores them on the heap. Anyway, in Java it's definitely going to grab more memory on the heap every time you call new MyBigObject(); Eventually the GC will kick in to keep you from running out of heap, though.



  • @boomzilla said:

    Filed under: I'm not changing it to goatsee either

    Not goatse, this:



  • @TheCPUWizard said:

    @Kattman -  Declaring a variable "just before the Try statement" is NOT the same thing as "at the beginning of the method". There are many valid use cases for the former (Dispose and using is a Red Herring), but not for the latter (consider that the try statement could be on line 50 of the method!)

    I agree, and this is where it comes from as so many are missing.  The point being, sometimes it is appropriate to declare and initialize on two separate lines considering scope.  The other poster that said "Hey look!" because he went with Using also is trying to just poke the bear when hopefully he understands he is totally wrong in the case I am talking about.

    Yes using will dispose, but what if it is the variable you are returning?  What if you need to clean up something in the catch?  Using will prevent this as I stated before because you are out of scope.  And once again before somone else tries being an idiot, this is not the common case, most times yes, declare and instantiate together, use using when you can, but in those cases where you need something outside of a limited scope, then declare first, then instantiate near where you are using it.  It doesn't matter if the try is 50 lines in or right at the top, if you need to reference the variable across a broader scope then give yourself the ability to do so.



  • @morbiuswilters said:

    @boomzilla said:
    The time drag is calling the constructor to allocate and initialize a new instance, not the storage space for x itself. That is presumably something just sitting around on the stack (or equivalent).

    Does C# store objects on the stack under certain conditions? Because, last time I checked, Java always stores them on the heap. Anyway, in Java it's definitely going to grab more memory on the heap every time you call new MyBigObject(); Eventually the GC will kick in to keep you from running out of heap, though.

    .NET (in current impementation and all previous) strores Reference Types on the Heap, Value Types and the items that HOLD the reference are stored "in the context of their containing scope". So local valuetypes and reference type variable ar on eht stack.



  • @KattMan said:

    Yes using will dispose, but what if it is the variable you are returning? 

    using(var x = new object())

    {

      return x;

    }

    @KattMan said:

    And once again before somone else tries being an idiot

    You've got it nailed pretty good yourself.@KattMan said:

    The other poster that said "Hey look!" because he went with Using also is trying to just poke the bear when hopefully he understands he is totally wrong in the case I am talking about.
    Hmmm, that's funny, because here's what I said

    @Sutherlands said:

    Declaring stuff outside a try/catch is fine if you need it, but that's not the case here, and is also not the case for calling Dispose...

     

    Since you apparently can't say what you actually mean, or don't know what you actually mean, or are just ignorant, let's go over what you said in your original post on the subject:

    @KattMan said:

    When a variable is assigned above the try it is seen within the catch and finally blocks for cleanup, setting to nothing if needed, and calling dispose in case of an error.  If the variable is declared within the try block, then it is out of scope in the catch and finally.  So in cases like this you want to declare on one line above the try, but assign within the try.  This does not apply to simple types such as strings or integers or really anything that doesn't need a dispose.

    You don't need to clean up a variable in the finally, as I've shown.  You don't need to set it to nothing.  It's already calling dispose.

    The only reason you would want to declare the variable outside of the try is if you need to access it in the catch for something OTHER than calling dispose, perhaps logging (or if you need it later in the function).

     

    Also, the OP said "here are lines right after the other where they declare it and set to null, then set it to something else."  You continued with "I see one reason to split them, and it is a good one."  Your reason then had nothing to do with the code in the OP.  It had to do with completely separate code.  There is no reason to do the code in the OP.  Your post was just a complete red herring from the beginning.



  • @TheCPUWizard said:

     Name one language (must be at least semi-mainstream, and less than 10 years since the latest version) that does that.

    You may have stopped reading my original post on this after the first sentence.  Here's the second sentence:

    @Sutherlands said:

    I don't know what those are or how true it is, but it's certainly not true about C#. 

    It may be a legacy from 20 years ago for all I know.  I've simply heard it as the reason to do it.  My team lead did it when I first joined the team I'm on.



  • @KattMan said:

    Yes using will dispose, but what if it is the variable you are returning? 

    Ummm....why would you ever return a disposed instance???



  • @Sutherlands said:

    @TheCPUWizard said:

     Name one language (must be at least semi-mainstream, and less than 10 years since the latest version) that does that.

    You may have stopped reading my original post on this after the first sentence.  Here's the second sentence:

    @Sutherlands said:

    I don't know what those are or how true it is, but it's certainly not true about C#. 

    It may be a legacy from 20 years ago for all I know.  I've simply heard it as the reason to do it.  My team lead did it when I first joined the team I'm on.

    "I've simply heard it as the reason to do it." - So you are a Cargo Cultist then.

    "My team lead did it when I first joined the team I'm on." - ALL practices should be reviewed on a regular basis. Failure to do so is the root cause of many a WTF. There have been multiple times where this conversation has occured:

    Me: "Why are you doing X?"
    Other: "Because that is the way we have always done it."
    Me: "One more time, what is the current reason?"
    Other: "Just as I said"
    Me: "Well you wont be doing it any more, at least not around here..enjoy your new job when you find it...good day."



  • @TheCPUWizard said:

    @Sutherlands said:

    @TheCPUWizard said:

     Name one language (must be at least semi-mainstream, and less than 10 years since the latest version) that does that.

    You may have stopped reading my original post on this after the first sentence.  Here's the second sentence:

    @Sutherlands said:

    I don't know what those are or how true it is, but it's certainly not true about C#. 

    It may be a legacy from 20 years ago for all I know.  I've simply heard it as the reason to do it.  My team lead did it when I first joined the team I'm on.

    "I've simply heard it as the reason to do it." - So you are a Cargo Cultist then.

    "My team lead did it when I first joined the team I'm on." - ALL practices should be reviewed on a regular basis. Failure to do so is the root cause of many a WTF. There have been multiple times where this conversation has occured:

    Me: "Why are you doing X?"
    Other: "Because that is the way we have always done it."
    Me: "One more time, what is the current reason?"
    Other: "Just as I said"
    Me: "Well you wont be doing it any more, at least not around here..enjoy your new job when you find it...good day."

    Wow, you sure think you know a lot about me.  I'm the one who got the team to stop doing that.  I also suggested when I was on the team before that to use consts and inline functions instead of #DEFINEs (in C++).  What you apparently fail to grasp is not everyone is in the position of power that you have with your 30 years experience, or whatever it is.  Some of us don't have the power to say "you won't be doing it anymore" and instead are in the position where people tell me (this actually happened) "that's how we do it.  Give it a few months and then if you want to start making suggestions we'll talk about them."  But thanks for the advice, I'll be sure to listen to it... well, never, because I already do what I can do improve the practices of my team.




  • @blakeyrat said:



  • @Sutherlands said:

    Wow, you sure think you know a lot about me.  I'm the one who got the team to stop doing that.  I also suggested when I was on the team before that to use consts and inline functions instead of #DEFINEs (in C++).  What you apparently fail to grasp is not everyone is in the position of power that you have with your 30 years experience, or whatever it is.  Some of us don't have the power to say "you won't be doing it anymore" and instead are in the position where people tell me (this actually happened) "that's how we do it.  Give it a few months and then if you want to start making suggestions we'll talk about them."  But thanks for the advice, I'll be sure to listen to it... well, never, because I already do what I can do improve the practices of my team.

    I do not know anything about you (except from that wou have posted here), and I can not concieve of a reason for you to think otherwise. You are correct that "Some of us don't have the power to say "you won't be doing it anymore"", in fact, I would think that most do not. I am indeed fortunate that I own the company where I do [and this has very little to do with how much experience I have today, as I formed the company when I only had about 8 years of professional experience].

    But, everyone does have the power to say "I will not be doing that" (unless an indentured person on some third world country) Yes there will be ramifications, especially in todays market, but it comes down to a persons beliefs and ethics. I am not saying that everyone "should" take such a position, simply that it is their choice.


  • Discourse touched me in a no-no place

    @morbiuswilters said:

    That's a double negative.


  • :belt_onion:

    @Sutherlands said:

    try

    {

      using(RecordSet rs = new RecordSet)

      {

        //Do stuff

      }

    }

    catch(Exception)

    {

      //Who cares? It's an example.

    }

     

    Hey look, i managed to dispose it without declaring it outside the try.

    Not sure what you are trying to prove here. "using" is nothing but syntactic sugar and in IL your code is equivalent to
    try
    {
      RecordSet rs = new RecordSet;
      try {
        //Do stuff
      }
      finally {
        rs.Dispose();
      }
    

    }
    catch(Exception)
    {

    //Who cares? It's an example.
    }

    So your still have declared your recordset outside the try block that handles the dispose. Only you have introduced 2 try blocks now.


  • ♿ (Parody)

    @TheCPUWizard said:

    @morbiuswilters said:
    @boomzilla said:
    The time drag is calling the constructor to allocate and initialize a new instance, not the storage space for x itself. That is presumably something just sitting around on the stack (or equivalent).

    Does C# store objects on the stack under certain conditions? Because, last time I checked, Java always stores them on the heap. Anyway, in Java it's definitely going to grab more memory on the heap every time you call new MyBigObject(); Eventually the GC will kick in to keep you from running out of heap, though.


    .NET (in current impementation and all previous) strores Reference Types on the Heap, Value Types and the items that HOLD the reference are stored "in the context of their containing scope". So local valuetypes and reference type variable ar on eht stack.

    Yes, that was my point. Basically that the reference (probably a pointer, of course, but that's just an implementation detail) held by x will be on the stack. The "storage space for x" was just the reference. Obviously, value type stuff (in languages that have that sort of thing) will complicate matters, but is out of scope of this particular conversation.



  • @blakeyrat said:


    +1



  • @PJH said:

    @morbiuswilters said:
    That's a double negative.

    I don't get it, unless yo are counting the "n" and the"t" separately because there's an apostrophe between them...


  • ♿ (Parody)

    @rstinejr said:

    @PJH said:
    @morbiuswilters said:
    That's a double negative.

    I don't get it, unless yo are counting the "n" and the"t" separately because there's an apostrophe between them...

    I suppose that might make sense if you were red-white colorblind.



  • @boomzilla said:

    @rstinejr said:
    @PJH said:
    @morbiuswilters said:
    That's a double negative.

    I don't get it, unless yo are counting the "n" and the"t" separately because there's an apostrophe between them...

    I suppose that might make sense if you were red-white colorblind.


    Red?

    D'oh!


  • Discourse touched me in a no-no place

    @rstinejr said:

    @PJH said:
    @morbiuswilters said:
    That's a double negative.

    I don't get it, unless yo are counting the "n" and the"t" separately because there's an apostrophe between them...
    The message says 'Don't Blindly Follow Convention.'



    The red circle with diagonal stripe indicates 'don't' or 'no' making the message "Don't, Don't Blindly Follow Convention."



    Which simplified translates as the sign saying "Blindly Follow Convention."



  • @PJH said:

    @morbiuswilters said:
    That's a double negative.

    "Hey, Ringo, that was the joke!"



  • @Sutherlands said:

    @KattMan said:

    Yes using will dispose, but what if it is the variable you are returning? 

    using(var x = new object())

    {

      return x;

    }

    <

    Not only will that not compile (System.Object does not have a Dispose method), but writing code like this is justifcation to break out the stabbings at my place of work.



  • @bjolling said:

    So your still have declared your recordset outside the try block that handles the dispose. Only you have introduced 2 try blocks now.

    No, I haven't.  I wrote a using block and correctly used it to clean up the resources of the recordset while making the code easy to read.  I don't give a flying crap what the IL does, or whether it's the same as what you wrote.  It was stated "you will need to declare the record set before the try in order to reference it in the finally in order to dispose."  I showed that, no, you don't, and I wrote code that did it the correct way.  Are you dense?


  • @pkmnfrk said:

    @Sutherlands said:

    @KattMan said:

    Yes using will dispose, but what if it is the variable you are returning? 

    using(var x = new object())

    {

      return x;

    }

    like this is justifcation to break out the stabbings at my place of work.
    Quoting fail.  I'm not sure what you're saying, exactly, but you make me look at it and say "this isn't even a valid scenario, since it will dispose of the object and you won't be able to use it."


  • @Sutherlands said:

    Quoting fail.

    Right, my post goes on the outside of the quote tags. Let's pretend it looked something like this:

    @Sutherlands said:

    @pkmnfrk said:

    @Sutherlands said:

    @KattMan said:

    Yes using will dispose, but what if it is the variable you are returning? 

    using(var x = new object())

    {

      return x;

    }

    Not only will this not compile (System.Object doesn't have a Dispose method), but writing code like this is justifcation to break out the stabbings at my place of work.

    I'm not sure what you're saying, exactly, but you make me look at it and say "this isn't even a valid scenario, since it will dispose of the object and you won't be able to use it."

    It's a perfectly valid scenario, from the compiler's point of view. The compiler will happily ensure that Dispose is called on the object just before it gets returned. If you need to return IDisposable objects, you don't put them in using blocks.



  • @PJH said:

    @morbiuswilters said:
    That's a double negative.

    Shit. Okay, here:



  • @pkmnfrk said:

    If you need to return IDisposable objects, you don't put them in using blocks.
    Right, so you wouldn't have to declare them outside whatever try/catch you have, because you wouldn't be disposing of them.


Log in to reply