Object/Procedure-Orientation
-
I keep seeing this pattern:
/** * This stores reference data type to the correct persistant storage. This was made * a separate method from store(Object o) because this must be able to accept a null * in order to unlock a table. * @param o The Data object that is to be stored * @param stomp boolean indicating whether or not a piece of Data is to be * overwritten if it already exists in the database * @throws UneditableStateExceptoion if the Data object is checked out or if the Data * object already exists if the stomp parameter was false * @return boolean indicating if the store was successful */ public boolean storeData(Object o, boolean stomp) throws UneditableStateException {boolean rtn = false;
if (o instanceof DataType1) {
rtn = storeDataType1((DataType1) o, stomp);} else if ( o instanceof DataType2 ) {
rtn = storeDataType2((DataType2) o, stomp);} else if ( o instanceof DataType3 ) {
rtn = storeDataType3((DataType3) o, stomp);} else if ( o instanceof DataType4 ) {
rtn = storeDataType4((DataType4) o);} else if ( o instanceof DataType5 ) {
rtn = storeDataType5((DataType5) o);}
unlockDataTables();
rtn = true;return rtn;
}
What does unlockDataTables() do? It goes to a LOCKS table ( table_name (varchar2), locked (varchar2), last_modified (date) ), counts the number of entries for table_name = 'DATA_TABLES'.
If the count != 1, then it deletes all entries for table_name = 'DATA_TABLES' then inserts a new record for 'DATA_TABLES', setting unlocked = 'Unlocked' and last_modified = sysdate
If the count == 1, then it updates all entries for table_name = 'DATA_TABLES', , setting unlocked = 'Unlocked' and last_modified = sysdate
If a SQLException is thrown during the insert/delete or update above, the catch statement tries to delete all entries for table_name = 'DATA_TABLES' then inserts a new record for 'DATA_TABLES', setting unlocked = 'Unlocked' and last_modified = sysdate (the action for if count != 1)
-
I think I just had a seizure...
So this table doesn't actually "lock" anything... Beyond maintaining only one record in the "lock" table, does it actually check to see if the table is locked before it does its business?
-
Apart from the stupid idea that if sequence is and the insanity of unlockDataTables(), I really love this pattern:
boolean rtn = false; // Snip block that might alter rtn, but never returns or uses the value of rtn rtn = true; return rtn;
-
@derula said:
Apart from the stupid idea that if sequence is and the insanity of unlockDataTables(), I really love this pattern:
That's actually very useful and sensible. When you're debugging the code, you can tell whether it's reached the end of the method or not by checking the value of rtn.boolean rtn = false;
// Snip block that might alter rtn, but never returns or uses the value of rtn
rtn = true;
return rtn;
-
@Iago said:
@derula said:
Apart from the stupid idea that if sequence is and the insanity of unlockDataTables(), I really love this pattern:
That's actually very useful and sensible. When you're debugging the code, you can tell whether it's reached the end of the method or not by checking the value of rtn.boolean rtn = false;
// Snip block that might alter rtn, but never returns or uses the value of rtn
rtn = true;
return rtn;
The equivalent of the old "necktie on the dormroom doorknob", eh?
-
@C-Octothorpe said:
I think I just had a seizure...
Welcome to my world. ;)
So this table doesn't actually "lock" anything...
Nope. Only a place for the code to keep track of which method/object/whatever has exclusive access to the various tables.
Beyond maintaining only one record in the "lock" table, does it actually check to see if the table is locked before it does its business?
Obviously, not in this instance.
It is used in the occasional place, though. Not wide-ranged enough to actually work reliably, though. Happily, these tables are rarely written to; fairly static, read-only things.
-
@Iago said:
@derula said:
Apart from the stupid idea that if sequence is and the insanity of unlockDataTables(), I really love this pattern:
boolean rtn = false;
// Snip block that might alter rtn, but never returns or uses the value of rtn
rtn = true;
return rtn;
That's actually very useful and sensible. When you're debugging the code, you can tell whether it's reached the end of the method or not by checking the value of rtn.Yes, the pattern itself is fine. But I hate the name rtn. At least call it ok, or something like that. Even worse when the return type is something other than boolean.
-
@boomzilla said:
@Iago said:
No it's not. He was being facetious. When you're debugging, you know whether it has reached the end by whether it has reached the end. This return value is absolutely useless, and should be replaced with "return false"@derula said:
Yes, the pattern itself is fine. But I hate the name rtn. At least call it ok, or something like that. Even worse when the return type is something other than boolean.Apart from the stupid idea that if sequence is and the insanity of unlockDataTables(), I really love this pattern:
That's actually very useful and sensible. When you're debugging the code, you can tell whether it's reached the end of the method or not by checking the value of rtn.boolean rtn = false;
// Snip block that might alter rtn, but never returns or uses the value of rtn
rtn = true;
return rtn;
-
@Sutherlands said:
@boomzilla said:
Yes, the pattern itself is fine. But I hate the name rtn. At least call it ok, or something like that. Even worse when the return type is something other than boolean.
No it's not. He was being facetious. When you're debugging, you know whether it has reached the end by whether it has reached the end. This return value is absolutely useless, and should be replaced with "return false"
Eh...yeah, in this example, that's absolutely true...but only if you actually read what people wrote, and why would I bother with that? Nevertheless, I stand by my comment about "rtn" being a stupid-ass name.
-
@boomzilla said:
I stand by my comment about "rtn" being a stupid-ass name.
That is of course true. It should be '@return'.
-
@Sutherlands said:
@boomzilla said:
I stand by my comment about "rtn" being a stupid-ass name.
That is of course true. It should be '@return'.I hope you're not advocating ending a sub with
return @return == 1 ? $return[0] : @return;. Because I've seen enough of that particular pattern.(For those non-perl programmers out there, the less likely to surprise version would be
return wantarray ? @return : $return[0].)Disclaimer: this post assumes perl5. I understand both my examples do not compile correctly in perl6.
-
@tgape said:
stuff about perl
No, no, no! I am, of course, assuming C#.
What's the use/meaning of the @ character in variable names in C#?
I discovered that you can start your variable name with a '@' character in C#. In my C# project I was using a web service (I added a web reference to my project) that was written in Java. One of the
-
@boomzilla said:
@Sutherlands said:
@boomzilla said:
Eh...yeah, in this example, that's absolutely true...but only if you actually read what people wrote, and why would I bother with that? Nevertheless, I stand by my comment about "rtn" being a stupid-ass name.Yes, the pattern itself is fine. But I hate the name rtn. At least call it ok, or something like that. Even worse when the return type is something other than boolean.
No it's not. He was being facetious. When you're debugging, you know whether it has reached the end by whether it has reached the end. This return value is absolutely useless, and should be replaced with "return false"I usually see that pattern in old code where the business logic has changed. The code will mix up data manipulation with validation, and thus Things Happen even when the operation fails. Or it does something like "Save the record, then return True / False if the client's allocation is full".
Some time later, the business logic is changed. The concept of "max records / client" is eliminated, for example. So now that function needs to return true all the time. Rather than recoding the function, which would take time, risk breaking something else, and (more likely) is beyond the skill of the current maintainer, it is easier to just slap "rtn = true" just prior to the return. Crap looking code, but at least it is also documented how it used to work, for when the client changes their mind again.
As for "rtn", yeah, that's stupid. I prefer "rv", for Return Value. It isn't hungarian, and if I need to do some copy pasta, I don't need to change rvClientAllocationAvailable to rv_IsHamsterExternallyLocated.
-
@Lorne Kates said:
As for "rtn", yeah, that's stupid. I prefer "rv", for Return Value. It isn't hungarian, and if I need to do some copy pasta, I don't need to change rvClientAllocationAvailable to rv_IsHamsterExternallyLocated.
The only problem that I have with variable names such as rtn, or even rv, is that it does leave room for misinterpretation. If somebody who is unfamiliar with your code starts skimming through, they have to stop and process the possible meaning of the variable name. I can understand that there used to be constraints on how long a variable name is, and I can definitely understand a developer wanting to keep their variable names reasonable. But, to lop off a few characters seems silly to me. I personally use names such as $return, $result, and $results. To me the meaning is perfectly clear, especially with the singular and plural versions of $result(s). If it is singular, then expect a single value. If it is plural, then expect more than one. $return is simply used when I have a brain fart and can't think of a better variable name. Regardless, it definitely beats naming a variable specific to the function it's used in, especially if there is even the slightest chance of "copy pasta".
-
@dohpaz42 said:
The only problem that I have with variable names such as rtn, or even rv, is that it does leave room for misinterpretation. If somebody who is unfamiliar with your code starts skimming through, they have to stop and process the possible meaning of the variable name. I can understand that there used to be constraints on how long a variable name is, and I can definitely understand a developer wanting to keep their variable names reasonable. But, to lop off a few characters seems silly to me. I personally use names such as $return, $result, and $results. To me the meaning is perfectly clear, especially with the singular and plural versions of $result(s). If it is singular, then expect a single value. If it is plural, then expect more than one. $return is simply used when I have a brain fart and can't think of a better variable name.
You're not really providing much clarity by adding in those extra letters. It's still a stupid name.
@dohpaz42 said:
Regardless, it definitely beats naming a variable specific to the function it's used in, especially if there is even the slightest chance of "copy pasta".
Oh, sorry, I thought you were being serious before. Now I see that IHBT. Good one.
-
@boomzilla said:
You're not really providing much clarity by adding in those extra letters. It's still a stupid name.
I'm quite interested in your reasoning for why it's a stupid name. It's succinct, and it instantly defines the intended purpose of the variable. Or is it less about the name of the variable, and just how it's used (i.e., single point of exit, over multiple return statements)?
-
@dohpaz42 said:
@boomzilla said:
You're not really providing much clarity by adding in those extra letters. It's still a stupid name.
I'm quite interested in your reasoning for why it's a stupid name. It's succinct, and it instantly defines the intended purpose of the variable. Or is it less about the name of the variable, and just how it's used (i.e., single point of exit, over multiple return statements)?It tells you that you're going to return it, but that's it. I'd rather name it something with more meaning about what it is. For a boolean, something like ok. Compare:
rtrn &= checkSomething(); rtrn &= checkSomethingElse(); return rtrn; vs ok &= checkSomething(); ok &= checkSomethingElse(); return ok;I don't really care about single vs multiple exit points per se. I use them both where they work. It's just that "this is what I'm returning!" doesn't seem as meaningful to me as some semantic meaning about the variable. The method reads worse to me when you use rtrn (or whatever) all over the place. Having a real name makes it more cohesive to me.
-
@boomzilla said:
It tells you that you're going to return it, but that's it. I'd rather name it something with more meaning about what it is. For a boolean, something like ok. Compare:rtrn &= checkSomething(); rtrn &= checkSomethingElse(); return rtrn; vs ok &= checkSomething(); ok &= checkSomethingElse(); return ok;I don't really care about single vs multiple exit points per se. I use them both where they work. It's just that "this is what I'm returning!" doesn't seem as meaningful to me as some semantic meaning about the variable. The method reads worse to me when you use rtrn (or whatever) all over the place. Having a real name makes it more cohesive to me.
Eh, okay, I can understand your side of the argument. FWIW, it seems that we are talking about two distinct purposes here: I am arguing for the purpose of the variable, and you are arguing for its meaning. I can definitely see the merit in defining a variable name based on its meaning if it was used in the function as something more than just a return value; i.e., before returning the value, perform some extra logic based on its value. That definitely would make more sense. However, if the variable is nothing more than a return value, then why bother caring about the variable's meaning?
-
@dohpaz42 said:
However, if the variable is nothing more than a return value, then why bother caring about the variable's meaning?
It has to be something more than a return value. "The value to be returned" (which is what your name says) is simply what you're going to do with the variable. That's pretty obvious from the fact that it shows up right after the word return. It's no different than giving sensible names to all of your variables (simple loop iterators usually excepted).
-
Btw, I was being facetious before. I agree with boomzilla.
-
@boomzilla said:
giving sensible names to all of your variables (simple loop iterators usually excepted).
Not acceptable to the previous lead programmer here. Loop variables should ALWAYS have meaningful names. No 'i', 'j', 'k', etc.
But something silly like 'loopCount' is okay ... nested loops get particularly nasty to parse mentally ... 'outerLoopCount', 'innerLoopCount' ... and, if you're doing triple-nested loops: 'middelLoopCount' ...
'tis why I've tried getting my comrades to eschew 'old style' for loops and instead use 'for each' loops ... and the loop variable pretty much names itself nicely.
for (String name : names) { /* do stuff with name */ };or
for (Record record : records) { /* do stuff with record */ }
-
Well, yes... foreach loops are so much better. But Shirley, you can come up with better names than "innerLoopCount", such as "attributeCount".
-
@Sutherlands said:
Well, yes... foreach loops are so much better. But Shirley, you can come up with better names than "innerLoopCount", such as "attributeCount".
Of course! iLoop, jLoop, kLoop...
-
@zelmak said:
nested loops get particularly nasty to parse mentally ... 'outerLoopCount', 'innerLoopCount' ... and, if you're doing triple-nested loops: 'middelLoopCount' ...
The real fun is when you need to add another loop inside the inner loop. Of course, you're not stuck, in fact you have many choices: realInnerLoopCount, innerInnerLoopCount, mostInnerLoopCount, and so on.
-
Sadly, this same guy would use 'it' to name a Collection's returned Iterator.
:sigh:
