Skip to main content
added 111 characters in body
Source Link

Thanks to your method IndiaFlag, I guess you're drawing a flag of India. The problem is, I don't know what it looks like. OK, I gonna google it.

It consists of 3 stripes: orange, white and green (top to bottom) and a sort of a wheel in the center. So, why don't you just say:

int const width = ...
int const height = ...
int const stripeHeight = height / 3;

drawStripe(0, 0, width, stripeHeight, Orange);
drawStripe(0, stripeHeight, width, 2 * stripeHeight, White);
drawStripe(0, 2 * stripeHeight, width, 3 * stripeHeight, Green);
drawWheel(width / 2, height / 2)

?

This code describes what your're drawing in fact. There are no brushes or pencils here, because if I ask you what the flag looks like, for sure you're not going to tell me about pencils and brushes and how GDI works. You'll probably say there are 3 stripes and a wheel in the center and that's what I want to hear.

So, why do you write the code like you're not going to tell anyone what happens in there?

Looking at your code, I'd say that all the names you give are terrible:

CBrush brush(RGB(255,255,255));

What does it mean? You're only going to have a single brush, so that's why you call it a "brush"? Nothing specific about it? If I see a name brush 20 lines later, do you expect me to remember it is white or any specific at all?

CBrush *oldBrush = dc.SelectObject(&brush);

An old brush? When I see oldBrush 20 lines later, do you expect me to remember what this brush is? Do YOU know why you need this oldBrush? I see no code where you switch back to it, so why do you even do it?

    dc.Rectangle(x,(y+50),(x+200),(y+100));

What is x here? Why y+50? What is 500? What is 200?

    CPen pen2(PS_SOLID, 1,RGB(0,0,255));

And once again, something called pen2. Is this pen any specific, or you're just telling me you're using 2 pens? Should I care? Etc.

P.S. Procedures/Functions/Methods describe actions, so their names are traditionally DoWhat. Your IndiaFlag violates the principle, because it says What.

Thanks to your method IndiaFlag, I guess you're drawing a flag of India. The problem is, I don't know what it looks like. OK, I gonna google it.

It consists of 3 stripes: orange, white and green (top to bottom) and a sort of a wheel in the center. So, why don't you just say:

int const width = ...
int const height = ...
int const stripeHeight = height / 3;

drawStripe(0, 0, width, stripeHeight, Orange);
drawStripe(0, stripeHeight, width, 2 * stripeHeight, White);
drawStripe(0, 2 * stripeHeight, width, 3 * stripeHeight, Green);
drawWheel(width / 2, height / 2)

?

This code describes what your're drawing in fact. There are no brushes or pencils here, because if I ask you what the flag looks like, for sure you're not going to tell me about pencils and brushes and how GDI works. You'll probably say there are 3 stripes and a wheel in the center and that's what I want to hear.

So, why do you write the code like you're not going to tell anyone what happens in there?

Looking at your code, I'd say that all the names you give are terrible:

CBrush brush(RGB(255,255,255));

What does it mean? You're only going to have a single brush, so that's why you call it a "brush"? Nothing specific about it? If I see a name brush 20 lines later, do you expect me to remember it is white or any specific at all?

CBrush *oldBrush = dc.SelectObject(&brush);

An old brush? When I see oldBrush 20 lines later, do you expect me to remember what this brush is?

    dc.Rectangle(x,(y+50),(x+200),(y+100));

What is x here? Why y+50? What is 500? What is 200?

    CPen pen2(PS_SOLID, 1,RGB(0,0,255));

And once again, something called pen2. Is this pen any specific, or you're just telling me you're using 2 pens? Should I care? Etc.

P.S. Procedures/Functions/Methods describe actions, so their names are traditionally DoWhat. Your IndiaFlag violates the principle, because it says What.

Thanks to your method IndiaFlag, I guess you're drawing a flag of India. The problem is, I don't know what it looks like. OK, I gonna google it.

It consists of 3 stripes: orange, white and green (top to bottom) and a sort of a wheel in the center. So, why don't you just say:

int const width = ...
int const height = ...
int const stripeHeight = height / 3;

drawStripe(0, 0, width, stripeHeight, Orange);
drawStripe(0, stripeHeight, width, 2 * stripeHeight, White);
drawStripe(0, 2 * stripeHeight, width, 3 * stripeHeight, Green);
drawWheel(width / 2, height / 2)

?

This code describes what your're drawing in fact. There are no brushes or pencils here, because if I ask you what the flag looks like, for sure you're not going to tell me about pencils and brushes and how GDI works. You'll probably say there are 3 stripes and a wheel in the center and that's what I want to hear.

So, why do you write the code like you're not going to tell anyone what happens in there?

Looking at your code, I'd say that all the names you give are terrible:

CBrush brush(RGB(255,255,255));

What does it mean? You're only going to have a single brush, so that's why you call it a "brush"? Nothing specific about it? If I see a name brush 20 lines later, do you expect me to remember it is white or any specific at all?

CBrush *oldBrush = dc.SelectObject(&brush);

An old brush? When I see oldBrush 20 lines later, do you expect me to remember what this brush is? Do YOU know why you need this oldBrush? I see no code where you switch back to it, so why do you even do it?

    dc.Rectangle(x,(y+50),(x+200),(y+100));

What is x here? Why y+50? What is 500? What is 200?

    CPen pen2(PS_SOLID, 1,RGB(0,0,255));

And once again, something called pen2. Is this pen any specific, or you're just telling me you're using 2 pens? Should I care? Etc.

P.S. Procedures/Functions/Methods describe actions, so their names are traditionally DoWhat. Your IndiaFlag violates the principle, because it says What.

added 803 characters in body
Source Link

Thanks to your method IndiaFlag, I guess you're drawing a flag of India. The problem is, I don't know what it looks like. OK, I gonna google it.

It consists of 3 stripes: orange, white and green (top to bottom) and a sort of a wheel in the center. So, why don't you just say:

int const width = ...
int const height = ...
int const stripeHeight = height / 3;

drawStripe(0, 0, width, stripeHeight, Orange);
drawStripe(0, stripeHeight, width, 2 * stripeHeight, White);
drawStripe(0, 2 * stripeHeight, width, 3 * stripeHeight, Green);
drawWheel(width / 2, height / 2)

?

This code describes what your're drawing in fact. There are notno brushes or pencils here, because if I ask you what the flag looks like, for sure you're not going to tell me about pencils and brushes and how GDI works. You'll probably say there are 3 stripes and a wheel in the center and that's what I want to hear.

So, why do you write the code like you're not going to tell anyone what happens in there?

Looking at your code, I'd say that all the names you give are terrible:

CBrush brush(RGB(255,255,255));

What does it mean? You're only going to have a single brush, so that's why you call it a "brush"? Nothing specific about it? If I see a name brush 20 lines later, do you expect me to remember it is white or any specific at all?

CBrush *oldBrush = dc.SelectObject(&brush);

An old brush? When I see oldBrush 20 lines later, do you expect me to remember what this brush is?

    dc.Rectangle(x,(y+50),(x+200),(y+100));

What is x here? Why y+50? What is 500? What is 200?

    CPen pen2(PS_SOLID, 1,RGB(0,0,255));

And once again, something called pen2. Is this pen any specific, or you're just telling me you're using 2 pens? Should I care? Etc.

P.S. Procedures/Functions/Methods describe actions, so their names are traditionally DoWhat. Your IndiaFlag violates the principle, because it says What.

Thanks to your method IndiaFlag, I guess you're drawing a flag of India. The problem is, I don't know what it looks like. OK, I gonna google it.

It consists of 3 stripes: orange, white and green (top to bottom) and a sort of a wheel in the center. So, why don't you just say:

int const width = ...
int const height = ...
int const stripeHeight = height / 3;

drawStripe(0, 0, width, stripeHeight, Orange);
drawStripe(0, stripeHeight, width, 2 * stripeHeight, White);
drawStripe(0, 2 * stripeHeight, width, 3 * stripeHeight, Green);
drawWheel(width / 2, height / 2)

?

This code describes what your're drawing in fact. There are not brushes or pencils here, because if I ask you what the flag looks like, for sure you're not going to tell me about pencils and brushes and how GDI works. You'll probably say there are 3 stripes and a wheel in the center and that's what I want to hear.

So, why do you write the code like you're not going to tell anyone what happens in there?

P.S. Procedures/Functions/Methods describe actions, so their names are traditionally DoWhat. Your IndiaFlag violates the principle, because it says What.

Thanks to your method IndiaFlag, I guess you're drawing a flag of India. The problem is, I don't know what it looks like. OK, I gonna google it.

It consists of 3 stripes: orange, white and green (top to bottom) and a sort of a wheel in the center. So, why don't you just say:

int const width = ...
int const height = ...
int const stripeHeight = height / 3;

drawStripe(0, 0, width, stripeHeight, Orange);
drawStripe(0, stripeHeight, width, 2 * stripeHeight, White);
drawStripe(0, 2 * stripeHeight, width, 3 * stripeHeight, Green);
drawWheel(width / 2, height / 2)

?

This code describes what your're drawing in fact. There are no brushes or pencils here, because if I ask you what the flag looks like, for sure you're not going to tell me about pencils and brushes and how GDI works. You'll probably say there are 3 stripes and a wheel in the center and that's what I want to hear.

So, why do you write the code like you're not going to tell anyone what happens in there?

Looking at your code, I'd say that all the names you give are terrible:

CBrush brush(RGB(255,255,255));

What does it mean? You're only going to have a single brush, so that's why you call it a "brush"? Nothing specific about it? If I see a name brush 20 lines later, do you expect me to remember it is white or any specific at all?

CBrush *oldBrush = dc.SelectObject(&brush);

An old brush? When I see oldBrush 20 lines later, do you expect me to remember what this brush is?

    dc.Rectangle(x,(y+50),(x+200),(y+100));

What is x here? Why y+50? What is 500? What is 200?

    CPen pen2(PS_SOLID, 1,RGB(0,0,255));

And once again, something called pen2. Is this pen any specific, or you're just telling me you're using 2 pens? Should I care? Etc.

P.S. Procedures/Functions/Methods describe actions, so their names are traditionally DoWhat. Your IndiaFlag violates the principle, because it says What.

Source Link

Thanks to your method IndiaFlag, I guess you're drawing a flag of India. The problem is, I don't know what it looks like. OK, I gonna google it.

It consists of 3 stripes: orange, white and green (top to bottom) and a sort of a wheel in the center. So, why don't you just say:

int const width = ...
int const height = ...
int const stripeHeight = height / 3;

drawStripe(0, 0, width, stripeHeight, Orange);
drawStripe(0, stripeHeight, width, 2 * stripeHeight, White);
drawStripe(0, 2 * stripeHeight, width, 3 * stripeHeight, Green);
drawWheel(width / 2, height / 2)

?

This code describes what your're drawing in fact. There are not brushes or pencils here, because if I ask you what the flag looks like, for sure you're not going to tell me about pencils and brushes and how GDI works. You'll probably say there are 3 stripes and a wheel in the center and that's what I want to hear.

So, why do you write the code like you're not going to tell anyone what happens in there?

P.S. Procedures/Functions/Methods describe actions, so their names are traditionally DoWhat. Your IndiaFlag violates the principle, because it says What.