|
I think you'll find the technical term is refuctoring.
|
|
|
|
|
While editing a much tweaked process that I wrote, I found this T-SQL:
DECLARE @NumDays INT;
SELECT @NumDays =
CASE
WHEN @Client = 'Client1' THEN @DaysBack
WHEN @Client = 'Client2' THEN @DaysBack
ELSE @DaysBack
END;
This is what happens when you go from actual numbers to a variable, without checking the code.
|
|
|
|
|
yeah you clearly missed the
@DaysBack
is this a signature ?
|
|
|
|
|
I've done something similar to this recently. It was like this:
SELECT CASE
WHEN Field1 = 'Something #1' THEN 'Result #1'
WHEN Field1 = 'Something #2' THEN 'N/A'
WHEN Field1 = 'Something #3' THEN '(Empty)'
ELSE 'N/A'
END
And I continued to wonder why even when 'Something #1' was not met, I was seeing 'N/A' in the results. Took me a bit to figure it out.
djj55: Nice but may have a permission problem
Pete O'Hanlon: He has my permission to run it.
|
|
|
|
|
djj55 wrote: What was I thinking
about Client3's perky assets?
|
|
|
|
|
My personal favorite from C#. Someone was testing the value of a boolean variable.
if (someBooleanVariable == true)
return true;
else
return false;
Later they refactored it to this thinking it was an improvement.
return someBooleanVariable == true ? true : false;
|
|
|
|
|
You mean they missed the totally obvious contraction to
return someBooleanVariable ? true : false
They should hang their head in shame!
cheers,
Chris Maunder
The Code Project | Co-founder
Microsoft C++ MVP
|
|
|
|
|
well, it should be just
return someBooleanVariable
|
|
|
|
|
cheers,
Chris Maunder
The Code Project | Co-founder
Microsoft C++ MVP
|
|
|
|
|
It took this long to get this simple return?
|
|
|
|
|
You're not following good form. You should use CONSTANTS for things that like.
return someBooleanVariable ? Boolean.Parse(Boolean.TrueString) : Boolean.Parse(Boolean.FalseString);
|
|
|
|
|
You mean
return someBooleanVariable.ToString() == Boolean.TrueString? Boolean.Parse(Boolean.TrueString) : Boolean.Parse(Boolean.FalseString);
Right?
cheers,
Chris Maunder
The Code Project | Co-founder
Microsoft C++ MVP
|
|
|
|
|
Good call.
|
|
|
|
|
Chris Maunder wrote: You mean they missed the totally obvious contraction to
return someBooleanVariable ? true : false
They should hang their head in shame!
No! Wrong!
return (someBooleanVariable == true)
|
|
|
|
|
Not good enough:
if (((Boolean)someBooleanVariable).equals(true)) {
return someBooleanVariable == true ? someBooleanVariable : false;
} else {
return someBooleanVariable == false ? someBooleanVariable : true;
}
it's called the Irish Method - to be sure, to be sure, to be sure.
Panic, Chaos, Destruction. My work here is done.
Drink. Get drunk. Fall over - P O'H
OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre
I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer
Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
|
|
|
|
|
G James wrote: Later they refactored it to this thinking it was an improvement.
return someBooleanVariable == true ? true : false;
That's pretty poor refactoring, it should have been:
interface IBooleanConverter {
bool BooleanToBoolean(bool value);
}
class BooleanConverter : IBooleanConverter {
public BooleanToBoolean(bool value) {
return value == true ? true : false;
}
}
interface IBooleanConverterFactory {
IBooleanConverter CreateBooleanConverterFactory();
}
class ConfigurationBooleanConverterFactory : IBooleanConverter {
public IBooleanConverter CreateBooleanConverter() {
return (IBooleanConverter)
Activator.CreateInstance(ConfigurationManager.AppSettings["Types.BooleanConverter"]);
}
}
static class BooleanConverterFactoryFactory {
public IBooleanConverterFactory CreateBooleanConverterFactory() {
return (IBooleanConverterFactory)
Activator.CreateInstance(ConfigurationManager.AppSettings["Types.BooleanConverterFactory"]);
}
}
return BooleanConverterFactoryFactory.CreateBooleanConverterFactory().CreateBooleanConverter().BooleanToBoolean(someBooleanVariable == true ? true : false);
It is clear that this is more maintainable. There is currently this method in our code-base because some [one] of our developers is so bad that we need to hold his hand through everything:
public static T As<T>(this object obj) {
return obj as T;
}
Yes guys, an extension method that replicates a keyword.
He who asks a question is a fool for five minutes. He who does not ask a question remains a fool forever. [Chinese Proverb]
Jonathan C Dickinson (C# Software Engineer)
|
|
|
|
|
my eyes bleed and my head hurts
it's so dificult to believe i'm seeing so well structured sh*t-code
I'm brazilian and english (well, human languages in general) aren't my best skill, so, sorry by my english. (if you want we can speak in C# or VB.Net =p)
|
|
|
|
|
Sentenryu wrote: it's so dificult to believe i'm seeing so well structured sh*t-code
And a lesson has been learnt about GOF design patterns .
He who asks a question is a fool for five minutes. He who does not ask a question remains a fool forever. [Chinese Proverb]
Jonathan C Dickinson (C# Software Engineer)
|
|
|
|
|
I hope you are not one of those gang of 4 guys
Ranjan.D
|
|
|
|
|
Ah, now I see what people mean by maintainable code - code that will require maintenance, and hence pay the bills for years to come. Genius.
|
|
|
|
|
This could make a good business sense when some decisions for different clients have not been made. Make the code structure ready and wait for the changes. I recently worked on a project in which different user role was supposed to have a different permission. But the decision was not made. Therefore all roles were assigned the same permission during development. Sometimes, the decision is never made. Several years later, when you look at the code, you would question why you did that.
TOMZ_KV
|
|
|
|
|
After my well deserved vacation I returned to work.
Beeing happy I received a task to modify a DotNetNuke module, made by some externals from a country far far away.
Happyness didn't last long when I came along a SearchEngine class:
public const string SELECT = "SELECT";
public const string DISTINCT = "DISTINCT";
public const string FROM = "FROM";
public const string WHERE = "WHERE";
public const string HAVING = "HAVING";
public const string AND = "AND";
public const string OR = "OR";
public const string IN = "IN";
public const string ON = "ON";
public const string AS = "AS";
public const string GROUP_BY = "GROUP BY";
public const string ORDER_BY = "ORDER BY";
public const string DESC = "DESC";
public const string ASC = "ASC";
this continues for about 1500 lines of code declaring every possible table/column etc I could think of.
90% of the consts aren't even in use... and it all ends up in statements like that:
sqlString.AppendFormat("{0} {1} {2}.{3},{2}.{4},{2}.{5},{2}.{6},{2}.{7},{2}.{8},{2}.{9},{2}.{10},{2}.{11},{12} {13} {14} {15} {2} {16} {17} = @{17}",new string[]
{
SqlConsts.SELECT,
SqlConsts.DISTINCT,
SqlConsts.TBL1,
SqlConsts.OBJECT_NO,
SqlConsts.OBJECT_ID,
SqlConsts.COUNTRY,
SqlConsts.REGION,
SqlConsts.SUBREGION,
SqlConsts.TOURAREA,
SqlConsts.LATITUDE,
SqlConsts.LONGITUDE,
SqlConsts.CITY,
SqlConsts.COUNT_ANY,
SqlConsts.AS,
SqlConsts.UNIT_COUNT,
SqlConsts.FROM,
SqlConsts.WHERE,
SqlConsts.YEAR
});
...am I missing something or is this an acceptable way of solving database related tasks?
Can you spot a reason for this? If so, please make my happiness return... somehow... please...
Happy new year
Andy
|
|
|
|
|
Great Zarquon! Thermite the disk immediately before that corrupts something else. Not even I would do that*.
There is absolutely nothing good about that and all sorts of bad. The worst thing I can think of now (no coffee yet) is that you can't copy and paste it to SSMS (or similar) to test it. Just below that would be lack of syntax colouring if available.
You just ruined my day. Thanks for sharing.
* If I did, I'd use an enumeration.
|
|
|
|
|
I like the "thermiting" idea of you...
And this is just a small snippet of the project... it's like a "Coding Horror" blog that came to live...
As a grown up man I am not ashamed of my tears
|
|
|
|
|
Good heavens. Shoot the purp!
|
|
|
|