What is Better Code in Your Opinion?
-
This:
//If the flag is on then if(READBIT(\*pGlobalFlags, uFlag)) { //Value is set to on bstrValue = "ON"; } else { //Value is set to off bstrValue = "OFF"; }
Or this:
bstrValue = READBIT(*pGlobalFlags, uFlag) ? "ON" : "OFF";
:confused:
Matt Gerrans
If I really wanted to do it right, I'd do something like:
bstrValue = GetBitStateAsStateString(READBIT(*pGlobalFlags, uFlag));
or, if you want to go for debuggable/more readable:
byte bit=READBIT(*pGlobalFlags, uFlag);
bstrValue = GetBitStateAsStateString(bit);This gives you at least a fighting chance to handle internationalization and centralizes the meaning of a bit when converted to a string. After all, "True" and "False" might be equally valid, so you might have:
bstrValue = GetBitStateAsTruthString(READBIT(*pGlobalFlags, uFlag));
And no, I don't write code like that usually myself! :) Marc
People are just notoriously impossible. --DavidCrow
There's NO excuse for not commenting your code. -- John Simmons / outlaw programmer
People who say that they will refactor their code later to make it "good" don't understand refactoring, nor the art and craft of programming. -- Josh Smith -
This:
//If the flag is on then if(READBIT(\*pGlobalFlags, uFlag)) { //Value is set to on bstrValue = "ON"; } else { //Value is set to off bstrValue = "OFF"; }
Or this:
bstrValue = READBIT(*pGlobalFlags, uFlag) ? "ON" : "OFF";
:confused:
Matt Gerrans
Second one IMO.
"For fifty bucks I'd put my face in their soup and blow." - George Costanza
~ Web SQL Utility - asp.net app to query Access, SQL server, MySQL. Stores history, favourites.
-
This:
//If the flag is on then if(READBIT(\*pGlobalFlags, uFlag)) { //Value is set to on bstrValue = "ON"; } else { //Value is set to off bstrValue = "OFF"; }
Or this:
bstrValue = READBIT(*pGlobalFlags, uFlag) ? "ON" : "OFF";
:confused:
Matt Gerrans
I started to think about, but I have noticed what is a programming question in the Lounge, then my opinion goes to soapbox... Ok, ok I'm lazy about select another forum... Here goes: The first option is better. Why? The code is easy to maintain. If I wish add some task if is true (by example), I can do this:
//If the flag is on then
if(READBIT(*pGlobalFlags, uFlag))
{
//Value is set to on
bstrValue = "ON";
// another task here
MyAnotherTaskFunction();
}
else
{
//Value is set to off
bstrValue = "OFF";
}Notice what I just added 1 line of code, and in your another style I will need create the supressed if construct. Just my 2 cents Regards
For God so loved the world, that he gave his only begotten Son, that whosoever believeth in him should not perish, but have everlasting life.(John 3:16) :badger:
-
This:
//If the flag is on then if(READBIT(\*pGlobalFlags, uFlag)) { //Value is set to on bstrValue = "ON"; } else { //Value is set to off bstrValue = "OFF"; }
Or this:
bstrValue = READBIT(*pGlobalFlags, uFlag) ? "ON" : "OFF";
:confused:
Matt Gerrans
// Return 'ON' or 'OFF' for the flag bstrValue = READBIT(*pGlobalFlags, uFlag) ? "ON" : "OFF";
No need for either extreme. -
This:
//If the flag is on then if(READBIT(\*pGlobalFlags, uFlag)) { //Value is set to on bstrValue = "ON"; } else { //Value is set to off bstrValue = "OFF"; }
Or this:
bstrValue = READBIT(*pGlobalFlags, uFlag) ? "ON" : "OFF";
:confused:
Matt Gerrans
They're both fine. I think the second should be used sparingly but there are situations in which it is very elegant.
Kevin
-
This:
//If the flag is on then if(READBIT(\*pGlobalFlags, uFlag)) { //Value is set to on bstrValue = "ON"; } else { //Value is set to off bstrValue = "OFF"; }
Or this:
bstrValue = READBIT(*pGlobalFlags, uFlag) ? "ON" : "OFF";
:confused:
Matt Gerrans
In my opinion, the latter. Conveys the same meaning as the former snippet, but with a whole lot less verbosity.
-- Made From Meat By-Products
-
I started to think about, but I have noticed what is a programming question in the Lounge, then my opinion goes to soapbox... Ok, ok I'm lazy about select another forum... Here goes: The first option is better. Why? The code is easy to maintain. If I wish add some task if is true (by example), I can do this:
//If the flag is on then
if(READBIT(*pGlobalFlags, uFlag))
{
//Value is set to on
bstrValue = "ON";
// another task here
MyAnotherTaskFunction();
}
else
{
//Value is set to off
bstrValue = "OFF";
}Notice what I just added 1 line of code, and in your another style I will need create the supressed if construct. Just my 2 cents Regards
For God so loved the world, that he gave his only begotten Son, that whosoever believeth in him should not perish, but have everlasting life.(John 3:16) :badger:
This wasn't really a programming question per se, but a programming aesthetics question, otherwise I would not have posed it here. What you are suggesting there is a nasty form of premature optimization[^] or Premature Generalization[^]. Are you saying you should write the most verbose slop possible, so that it might be easier to insert more stuff into it later? YAGNI[^]!
Matt Gerrans
-
// Return 'ON' or 'OFF' for the flag bstrValue = READBIT(*pGlobalFlags, uFlag) ? "ON" : "OFF";
No need for either extreme.But your comment is wrong! ;P (which cements a comment by someone else in this thread. Marc, I think it was).
Matt Gerrans
-
This:
//If the flag is on then if(READBIT(\*pGlobalFlags, uFlag)) { //Value is set to on bstrValue = "ON"; } else { //Value is set to off bstrValue = "OFF"; }
Or this:
bstrValue = READBIT(*pGlobalFlags, uFlag) ? "ON" : "OFF";
:confused:
Matt Gerrans
What happened to if(IsLounge) { Label1.Text="No Programming Questions"; } elseif { Label1.Text="Ask in appropriate forum"; }
A room without books is like a body without a soul. - Cicero (106 BC - 43 AD)
-
This:
//If the flag is on then if(READBIT(\*pGlobalFlags, uFlag)) { //Value is set to on bstrValue = "ON"; } else { //Value is set to off bstrValue = "OFF"; }
Or this:
bstrValue = READBIT(*pGlobalFlags, uFlag) ? "ON" : "OFF";
:confused:
Matt Gerrans
You're all wrong. The correct answer depends on how you're getting paid - by the number of lines written(then the first), or by the functionality implemented (then the second). Yup, thats it;)
:badger:
-
What happened to if(IsLounge) { Label1.Text="No Programming Questions"; } elseif { Label1.Text="Ask in appropriate forum"; }
A room without books is like a body without a soul. - Cicero (106 BC - 43 AD)
:P It's style question. People usually get away with it. IMO its equal to "Do you like code red or pitr-cola more?" kind of question.
"Throughout human history, we have been dependent on machines to survive. Fate, it seems, is not without a sense of irony. " - Morpheus
-
:P It's style question. People usually get away with it. IMO its equal to "Do you like code red or pitr-cola more?" kind of question.
"Throughout human history, we have been dependent on machines to survive. Fate, it seems, is not without a sense of irony. " - Morpheus
No, it is an aesthetics question! What programmer has style? ;)
Matt Gerrans
-
What happened to if(IsLounge) { Label1.Text="No Programming Questions"; } elseif { Label1.Text="Ask in appropriate forum"; }
A room without books is like a body without a soul. - Cicero (106 BC - 43 AD)
That's just plain wrong (and I'm not just talking about the superfluous curlies) -- I already pointed out that this is not about programming, per se. I guess some people can't tell the difference between a question and a metaquestion. ;P
Matt Gerrans
-
I started to think about, but I have noticed what is a programming question in the Lounge, then my opinion goes to soapbox... Ok, ok I'm lazy about select another forum... Here goes: The first option is better. Why? The code is easy to maintain. If I wish add some task if is true (by example), I can do this:
//If the flag is on then
if(READBIT(*pGlobalFlags, uFlag))
{
//Value is set to on
bstrValue = "ON";
// another task here
MyAnotherTaskFunction();
}
else
{
//Value is set to off
bstrValue = "OFF";
}Notice what I just added 1 line of code, and in your another style I will need create the supressed if construct. Just my 2 cents Regards
For God so loved the world, that he gave his only begotten Son, that whosoever believeth in him should not perish, but have everlasting life.(John 3:16) :badger:
Personally, I would do it like this, kinda taking best of both options (IMO).
// getting string value from flag
strValue = (READBIT(*pGlobalFlags, uFlag))?"ON":"OFF";// do different stuff depending on value of strValue
if(strValue == "ON")
{
MyTaskFunction();
}
else
{
MyAnotherTaskFunction();
}
"Throughout human history, we have been dependent on machines to survive. Fate, it seems, is not without a sense of irony. " - Morpheus
-
I started to think about, but I have noticed what is a programming question in the Lounge, then my opinion goes to soapbox... Ok, ok I'm lazy about select another forum... Here goes: The first option is better. Why? The code is easy to maintain. If I wish add some task if is true (by example), I can do this:
//If the flag is on then
if(READBIT(*pGlobalFlags, uFlag))
{
//Value is set to on
bstrValue = "ON";
// another task here
MyAnotherTaskFunction();
}
else
{
//Value is set to off
bstrValue = "OFF";
}Notice what I just added 1 line of code, and in your another style I will need create the supressed if construct. Just my 2 cents Regards
For God so loved the world, that he gave his only begotten Son, that whosoever believeth in him should not perish, but have everlasting life.(John 3:16) :badger:
Clickok wrote:
Notice what I just added 1 line of code, and in your another style I will need create the supressed if construct.
Yeah, but then you'll have your new line of code to focus on, and won't be as tempted to add useless comments to the code you're modifying...
-
This wasn't really a programming question per se, but a programming aesthetics question, otherwise I would not have posed it here. What you are suggesting there is a nasty form of premature optimization[^] or Premature Generalization[^]. Are you saying you should write the most verbose slop possible, so that it might be easier to insert more stuff into it later? YAGNI[^]!
Matt Gerrans
Matt Gerrans wrote:
This wasn't really a programming question per se, but a programming aesthetics question,
Just kidding with you ;)
Matt Gerrans wrote:
Are you saying you should write the most verbose slop possible
Not, I'm not saying this. I'm saying what I write code what...
Matt Gerrans wrote:
...might be easier to insert more stuff into it later
The comments what you added really are not necessary, because of the code is self-explanatory. But if I can code some snippet what later will be more easy to customize (like never hard-code strings, if possible declarate variables in beginning of procedures, etc), I will do. It's my habit. And really really turn the things more easy to me :)
For God so loved the world, that he gave his only begotten Son, that whosoever believeth in him should not perish, but have everlasting life.(John 3:16) :badger:
-
No, it is an aesthetics question! What programmer has style? ;)
Matt Gerrans
OK my bad. What about
bstrValue =
READBIT
(
*pGlobalFlags, uFlag
)
?
"ON"
:
"OFF";:rolleyes:
"Throughout human history, we have been dependent on machines to survive. Fate, it seems, is not without a sense of irony. " - Morpheus
-
This:
//If the flag is on then if(READBIT(\*pGlobalFlags, uFlag)) { //Value is set to on bstrValue = "ON"; } else { //Value is set to off bstrValue = "OFF"; }
Or this:
bstrValue = READBIT(*pGlobalFlags, uFlag) ? "ON" : "OFF";
:confused:
Matt Gerrans
I generally go with the first if I have more than one statement per condition.
Some people have a memory and an attention span, you should try them out one day. - Jeremy Falcon
-
That's just plain wrong (and I'm not just talking about the superfluous curlies) -- I already pointed out that this is not about programming, per se. I guess some people can't tell the difference between a question and a metaquestion. ;P
Matt Gerrans
Matt Gerrans wrote:
some people can't tell the difference between a question and a metaquestion.
Ha! metaquestion is very good! Do you accept metareplies too? :laugh::laugh::laugh:
For God so loved the world, that he gave his only begotten Son, that whosoever believeth in him should not perish, but have everlasting life.(John 3:16) :badger:
-
That's just plain wrong (and I'm not just talking about the superfluous curlies) -- I already pointed out that this is not about programming, per se. I guess some people can't tell the difference between a question and a metaquestion. ;P
Matt Gerrans
Rules: The Lounge is rated PG. If you're about to post something you wouldn't want your kid sister to read then don't post it. Do not post programming questions (use the programming forums for that) and please don't post ads. So sorry, you get it, no.
A room without books is like a body without a soul. - Cicero (106 BC - 43 AD)