Long Lines
-
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
-
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
I'd split it as you have done to improve readability and not worry about short-lived variables, the compiler optimisation will take care of that and in-line your code for you.
-
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
Strange. Much of my code looks like this:
Quote:
SEX R6 BN4 LOOP INP 04 OUT 04 B4 LOOP LDA R6 SDI 0F BNE LOOP NOP
The lines don't get too long, even if I use the old 64 x 64 pixel screen resolution. :-)
I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.
-
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
I'd just throw some newlines in there:
OutputImage(file,
allFaces.Where(f => f.Proportion > lowerBound &&
f.Proportion < upperBound).
ToList().
OrderBy(f => f.Proportion).
ThenByDescending(f => f.Rectangle.Height).
FirstOrDefault()); -
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
My personal style (which I suspect will not agree with anyone else's ideas) is ...
OutputImage(
file,
allFaces
.Where(
f =>
f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString())
&& f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())
)
.List()
.OrderBy(f => f.Proportion)
.ThenByDescending(f => f.Rectangle.Height)
.FirstOrDefault()
); -
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
Remember that the person who's doing the code review is a psychopath. :rolleyes: I'd split that bugger.
I'd rather be phishing!
-
My personal style (which I suspect will not agree with anyone else's ideas) is ...
OutputImage(
file,
allFaces
.Where(
f =>
f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString())
&& f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())
)
.List()
.OrderBy(f => f.Proportion)
.ThenByDescending(f => f.Rectangle.Height)
.FirstOrDefault()
); -
I'd just throw some newlines in there:
OutputImage(file,
allFaces.Where(f => f.Proportion > lowerBound &&
f.Proportion < upperBound).
ToList().
OrderBy(f => f.Proportion).
ThenByDescending(f => f.Rectangle.Height).
FirstOrDefault());That's the one I'd go with too. Except I'd put the dots on the new line:
OutputImage(file,
allFaces.Where(f => f.Proportion > lowerBound &&
f.Proportion < upperBound)
.ToList()
.OrderBy(f => f.Proportion)
.ThenByDescending(f => f.Rectangle.Height)
.FirstOrDefault());"I have no idea what I did, but I'm taking full credit for it." - ThisOldTony AntiTwitter: @DalekDave is now a follower!
-
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
I'd do nothing of the sort. Long-ass lines like that make the code harder to read, maintain, and debug. I'd bust the individual components out to their own variables and combine them in the final statement. When you compile for Release, all that gets optimized out in most cases. You're really not saving yourself anything by piling it all into a single statement.
Asking questions is a skill CodeProject Forum Guidelines Google: C# How to debug code Seriously, go read these articles.
Dave Kreskowiak -
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
PeejayAdams wrote:
Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good
You're not doing it for cosmetic reasons. You're doing it to improve code clarity. And that, IMHO, has huge value. /ravi
My new year resolution: 2048 x 1536 Home | Articles | My .NET bits | Freeware ravib(at)ravib(dot)com
-
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
I never used to worry about long lines but since I now use a wide screen monitor in portrait mode for editing I find scrolling right so annoying! In addition we used side by side code display for comparison during code reviews so shorter lines are better. Splitting after "(", ",", ".", "=>" and occasionally "=" really helps but keeps the code easily readable. I even adopted this style at home where there are no code reviewers beyond me.
- I would love to change the world, but they won’t give me the source code.
-
That's the one I'd go with too. Except I'd put the dots on the new line:
OutputImage(file,
allFaces.Where(f => f.Proportion > lowerBound &&
f.Proportion < upperBound)
.ToList()
.OrderBy(f => f.Proportion)
.ThenByDescending(f => f.Rectangle.Height)
.FirstOrDefault());"I have no idea what I did, but I'm taking full credit for it." - ThisOldTony AntiTwitter: @DalekDave is now a follower!
That's the way I do it. Except I move the && (or ||) to the start of the next line instead of leaving it trailing - it often helps when I want to comment out an option or two temporarily while testing.
- I would love to change the world, but they won’t give me the source code.
-
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());In my opinion this way would be better so that you can have error checking on it and provide a meaningful error message if one of the Appsettings is invalid.
Social Media - A platform that makes it easier for the crazies to find each other. Everyone is born right handed. Only the strongest overcome it. Fight for left-handed rights and hand equality.
-
Strange. Much of my code looks like this:
Quote:
SEX R6 BN4 LOOP INP 04 OUT 04 B4 LOOP LDA R6 SDI 0F BNE LOOP NOP
The lines don't get too long, even if I use the old 64 x 64 pixel screen resolution. :-)
I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.
-
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
Macros for the win. :cool:
-
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
Create an arguments class and pass that it. Then, when arguments changes (not 'if', but 'when'), then the method's signature stays the same.
If it's not broken, fix it until it is. Everything makes sense in someone's mind. Ya can't fix stupid.
-
I never used to worry about long lines but since I now use a wide screen monitor in portrait mode for editing I find scrolling right so annoying! In addition we used side by side code display for comparison during code reviews so shorter lines are better. Splitting after "(", ",", ".", "=>" and occasionally "=" really helps but keeps the code easily readable. I even adopted this style at home where there are no code reviewers beyond me.
- I would love to change the world, but they won’t give me the source code.
Splitting _before_ operators tends to help them stand out.
-
Splitting _before_ operators tends to help them stand out.
-
Just setting the X register to a new value. This means that the value in register 6 is now the new stack pointer.
I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.
-
Sometimes you find yourself writing a stupidly long line of code, in my case:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
That will auto-wrap on here but you get the idea. Well, obviously I can shorten that line (but lengthen the code slightly) by splitting out the config setting retrievals:
double lowerBound = double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString());
double upperBound = double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString());reducing the "offending" line to:
OutputImage(file, allFaces.Where(f => f.Proportion > lowerBound && f.Proportion < upperBound).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
But given that I'm not using those settings more than once, should I really be doing that? Two short-lived variables aren't going to make too much difference but I'm still doing it for cosmetic reasons and that can't be good ... Either way, it's easy enough to split a method call where there are half a ton of arguments over several lines of text but what about when it's just one of the arguments that's causing the thing to scroll for miles? (Lambdas being a common cause of this). Hardly a matter of life and death but I'd be interested to hear the thoughts of others on this.
Whenever you find yourself on the side of the majority, it is time to pause and reflect. - Mark Twain
PeejayAdams wrote:
Two short-lived variables
Lifetime should not influence readability. I almost always put things into variables not just for readability but that it makes it easier to debug. Given:
OutputImage(file, allFaces.Where(f => f.Proportion > double.Parse(ConfigurationManager.AppSettings["LowerBound"].ToString()) && f.Proportion < double.Parse(ConfigurationManager.AppSettings["UpperBound"].ToString())).ToList().OrderBy(f => f.Proportion).ThenByDescending(f => f.Rectangle.Height).FirstOrDefault());
I do have a penchant for extension methods. As in:
var face = allFaces
.AnyBetween(f => f.Proportion, "LowerBound".AppSetting().to_f(), "UpperBound".AppSetting().to_f())
.OrderBy(f => f.Proportion)
.ThenByDescending(f => f.Rectangle.Height)
.FirstOrDefault();
OutputImage(file, face);As bizarre as it is to extend
string, I find it a lot more readable. TheToList()seems superfluous. Not sure you want to output an image if no images meet the selection criteria. I might moveFirstOrDefaultintoOutputImage(file, face.FirstOrDefault());as well, who knows, the way you get the faces might be re-usable. I leave it to the reader to figure outto_f,AnyBetween, andAppSettingsShould be obvious.Latest Articles:
Client-Side TypeScript without ASP.NET, Angular, etc.
