###Naming
Naming
HemEnum
, or any enum name that ends with the word Enum
, is a bad name for an enum. Enum types should not contain the word "enum" in their names.
Similarly, HemEnum
values should not contain "Hem" in their names either. Your HemEnum
should therefore read something like this:
public enum HemType
{
None = -1,
Sewn = 0,
Weld = 1,
DoubleFold = 2
}
myObject
is a bad name for a class - should be MyObject
. Kidding (although not really - types should be named following a PascalCasing convention). Anything that ends with "Object" should be banned from being a class name. Note that myObject
doesn't compile as provided. (where's this.HemType
?)
The pseudo-Hungarian notation is ok in winforms for naming controls (i.e. the "cbo" prefix for ComboBoxes, "txt" for TextBoxes, etc.) - had you been using current technology (WPF) I would have strongly advised against such naming though.
###Nitpicks
Nitpicks
You seem to make static
anything that can be made static
. Don't. Just because a method doesn't use instance members now doesn't mean it never will, and changing a public static void
method to be public void
, is a breaking change.
vHemEnum
doesn't seem to be a type defined anywhere. Typo?
I'll assume the out-of-whack indentation is a Copy+Paste glitch.
###Binding?
Binding?
You say you want databinding, and yet you're doing this:
foreach (var value in values)
{
var s = GetHemTypeDescription((HemEnum)value.ID );
cbo.Items.Add(s);
}
Create a type that exposes DisplayValue
and EnumValue
properties, and use @DanLyons' or @JesseCSlicer's answer to bind your combobox items.
###Captions
Captions
A more extensible way (vs. attributes) of providing captions for your enums, would be to use a resource file (.resx) - call the strings per the enum's names, and then retrieve the caption from the resource strings:
var description = resx.ResourceManager.GetString(hemType.ToString());
Bottom line, I've seen worse WinForms code (and WPF for that matter), but it could be improved.
###Naming
HemEnum
, or any enum name that ends with the word Enum
, is a bad name for an enum. Enum types should not contain the word "enum" in their names.
Similarly, HemEnum
values should not contain "Hem" in their names either. Your HemEnum
should therefore read something like this:
public enum HemType
{
None = -1,
Sewn = 0,
Weld = 1,
DoubleFold = 2
}
myObject
is a bad name for a class - should be MyObject
. Kidding (although not really - types should be named following a PascalCasing convention). Anything that ends with "Object" should be banned from being a class name. Note that myObject
doesn't compile as provided. (where's this.HemType
?)
The pseudo-Hungarian notation is ok in winforms for naming controls (i.e. the "cbo" prefix for ComboBoxes, "txt" for TextBoxes, etc.) - had you been using current technology (WPF) I would have strongly advised against such naming though.
###Nitpicks
You seem to make static
anything that can be made static
. Don't. Just because a method doesn't use instance members now doesn't mean it never will, and changing a public static void
method to be public void
, is a breaking change.
vHemEnum
doesn't seem to be a type defined anywhere. Typo?
I'll assume the out-of-whack indentation is a Copy+Paste glitch.
###Binding?
You say you want databinding, and yet you're doing this:
foreach (var value in values)
{
var s = GetHemTypeDescription((HemEnum)value.ID );
cbo.Items.Add(s);
}
Create a type that exposes DisplayValue
and EnumValue
properties, and use @DanLyons' or @JesseCSlicer's answer to bind your combobox items.
###Captions
A more extensible way (vs. attributes) of providing captions for your enums, would be to use a resource file (.resx) - call the strings per the enum's names, and then retrieve the caption from the resource strings:
var description = resx.ResourceManager.GetString(hemType.ToString());
Bottom line, I've seen worse WinForms code (and WPF for that matter), but it could be improved.
Naming
HemEnum
, or any enum name that ends with the word Enum
, is a bad name for an enum. Enum types should not contain the word "enum" in their names.
Similarly, HemEnum
values should not contain "Hem" in their names either. Your HemEnum
should therefore read something like this:
public enum HemType
{
None = -1,
Sewn = 0,
Weld = 1,
DoubleFold = 2
}
myObject
is a bad name for a class - should be MyObject
. Kidding (although not really - types should be named following a PascalCasing convention). Anything that ends with "Object" should be banned from being a class name. Note that myObject
doesn't compile as provided. (where's this.HemType
?)
The pseudo-Hungarian notation is ok in winforms for naming controls (i.e. the "cbo" prefix for ComboBoxes, "txt" for TextBoxes, etc.) - had you been using current technology (WPF) I would have strongly advised against such naming though.
Nitpicks
You seem to make static
anything that can be made static
. Don't. Just because a method doesn't use instance members now doesn't mean it never will, and changing a public static void
method to be public void
, is a breaking change.
vHemEnum
doesn't seem to be a type defined anywhere. Typo?
I'll assume the out-of-whack indentation is a Copy+Paste glitch.
Binding?
You say you want databinding, and yet you're doing this:
foreach (var value in values)
{
var s = GetHemTypeDescription((HemEnum)value.ID );
cbo.Items.Add(s);
}
Create a type that exposes DisplayValue
and EnumValue
properties, and use @DanLyons' or @JesseCSlicer's answer to bind your combobox items.
Captions
A more extensible way (vs. attributes) of providing captions for your enums, would be to use a resource file (.resx) - call the strings per the enum's names, and then retrieve the caption from the resource strings:
var description = resx.ResourceManager.GetString(hemType.ToString());
Bottom line, I've seen worse WinForms code (and WPF for that matter), but it could be improved.
###Naming
HemEnum
, or any enum name that ends with the word Enum
, is a bad name for an enum. Enum types should not contain the word "enum" in their names.
Similarly, HemEnum
values should not contain "Hem" in their names either. Your HemEnum
should therefore read something like this:
public enum HemType
{
None = -1,
Sewn = 0,
Weld = 1,
DoubleFold = 2
}
myObject
is a bad name for a class - should be MyObject
. Kidding (although not really - types should be named following a PascalCasing convention). Anything that ends with "Object" should be banned from being a class name. Note that myObject
doesn't compile as provided. (where's this.HemType
?)
The pseudo-Hungarian notation is ok in winforms for naming controls (i.e. the "cbo" prefix for ComboBoxes, "txt" for TextBoxes, etc.) - had you been using current technology (WPF) I would have strongly advised against such naming though.
###Nitpicks
You seem to make static
anything that can be made static
. Don't. Just because a method doesn't use instance members now doesn't mean it never will, and changing a public static void
method to be public void
, is a breaking change.
vHemEnum
doesn't seem to be a type defined anywhere. Typo?
I'll assume the out-of-whack indentation is a Copy+Paste glitch.
###Binding?
You say you want databinding, and yet you're doing this:
foreach (var value in values)
{
var s = GetHemTypeDescription((HemEnum)value.ID );
cbo.Items.Add(s);
}
Create a type that exposes DisplayValue
and EnumValue
properties, and use @DanLyons' or @JesseCSlicer's answer to bind your combobox items.
###Captions
A more extensible way (vs. attributes) of providing captions for your enums, would be to use a resource file (.resx) - call the strings per the enum's names, and then retrieve the caption from the resource strings:
var description = resx.ResourceManager.GetString(hemType.ToString());
Bottom line, I've seen worse WinForms code (and WPF for that matter), but it could be improved.
###Naming
HemEnum
, or any enum name that ends with the word Enum
, is a bad name for an enum. Enum types should not contain the word "enum" in their names.
Similarly, HemEnum
values should not contain "Hem" in their names either. Your HemEnum
should therefore read something like this:
public enum HemType
{
None = -1,
Sewn = 0,
Weld = 1,
DoubleFold = 2
}
myObject
is a bad name for a class - should be MyObject
. Kidding (although not really - types should be named following a PascalCasing convention). Anything that ends with "Object" should be banned from being a class name. Note that myObject
doesn't compile as provided. (where's this.HemType
?)
The pseudo-Hungarian notation is ok in winforms for naming controls (i.e. the "cbo" prefix for ComboBoxes, "txt" for TextBoxes, etc.) - had you been using current technology (WPF) I would have strongly advised against such naming though.
###Nitpicks
You seem to make static
anything that can be made static
. Don't. Just because a method doesn't use instance members now doesn't mean it never will, and changing a public static void
method to be public void
, is a breaking change.
vHemEnum
doesn't seem to be a type defined anywhere. Typo?
I'll assume the out-of-whack indentation is a Copy+Paste glitch.
###Binding?
You say you want databinding, and yet you're doing this:
foreach (var value in values)
{
var s = GetHemTypeDescription((HemEnum)value.ID );
cbo.Items.Add(s);
}
Create a type that exposes DisplayValue
and EnumValue
properties, and use @DanLyons' answer to bind your combobox items.
###Captions
A more extensible way of providing captions for your enums, would be to use a resource file (.resx) - call the strings per the enum's names, and then retrieve the caption from the resource strings:
var description = resx.ResourceManager.GetString(hemType.ToString());
Bottom line, I've seen worse WinForms code (and WPF for that matter), but it could be improved.
###Naming
HemEnum
, or any enum name that ends with the word Enum
, is a bad name for an enum. Enum types should not contain the word "enum" in their names.
Similarly, HemEnum
values should not contain "Hem" in their names either. Your HemEnum
should therefore read something like this:
public enum HemType
{
None = -1,
Sewn = 0,
Weld = 1,
DoubleFold = 2
}
myObject
is a bad name for a class - should be MyObject
. Kidding (although not really - types should be named following a PascalCasing convention). Anything that ends with "Object" should be banned from being a class name. Note that myObject
doesn't compile as provided. (where's this.HemType
?)
The pseudo-Hungarian notation is ok in winforms for naming controls (i.e. the "cbo" prefix for ComboBoxes, "txt" for TextBoxes, etc.) - had you been using current technology (WPF) I would have strongly advised against such naming though.
###Nitpicks
You seem to make static
anything that can be made static
. Don't. Just because a method doesn't use instance members now doesn't mean it never will, and changing a public static void
method to be public void
, is a breaking change.
vHemEnum
doesn't seem to be a type defined anywhere. Typo?
I'll assume the out-of-whack indentation is a Copy+Paste glitch.
###Binding?
You say you want databinding, and yet you're doing this:
foreach (var value in values)
{
var s = GetHemTypeDescription((HemEnum)value.ID );
cbo.Items.Add(s);
}
Create a type that exposes DisplayValue
and EnumValue
properties, and use @DanLyons' or @JesseCSlicer's answer to bind your combobox items.
###Captions
A more extensible way (vs. attributes) of providing captions for your enums, would be to use a resource file (.resx) - call the strings per the enum's names, and then retrieve the caption from the resource strings:
var description = resx.ResourceManager.GetString(hemType.ToString());
Bottom line, I've seen worse WinForms code (and WPF for that matter), but it could be improved.
###Naming
HemEnum
, or any enum name that ends with the word Enum
, is a bad name for an enum. Enum types should not contain the word "enum" in their names.
Similarly, HemEnum
values should not contain "Hem" in their names either. Your HemEnum
should therefore read something like this:
public enum HemType
{
None = -1,
Sewn = 0,
Weld = 1,
DoubleFold = 2
}
myObject
is a bad name for a class - should be MyObject
. Kidding (although not really - types should be named following a PascalCasing convention). Anything that ends with "Object" should be banned from being a class name. Note that myObject
doesn't compile as provided. (where's this.HemType
?)
The pseudo-Hungarian notation is ok in winforms for naming controls (i.e. the "cbo" prefix for ComboBoxes, "txt" for TextBoxes, etc.) - had you been using current technology (WPF) I would have strongly advised against such naming though.
###Nitpicks
You seem to make static
anything that can be made static
. Don't. Just because a method doesn't use instance members now doesn't mean it never will, and changing a public static void
method to be public void
, is a breaking change.
vHemEnum
doesn't seem to be a type defined anywhere. Typo?
I'll assume the out-of-whack indentation is a Copy+Paste glitch.
###Binding?
You say you want databinding, and yet you're doing this:
foreach (var value in values)
{
var s = GetHemTypeDescription((HemEnum)value.ID );
cbo.Items.Add(s);
}
Create a type that exposes DisplayValue
and EnumValue
properties, and use @DanLyons' answer to bind your combobox items.
###Captions
A more extensible way of providing captions for your enums, would be to use a resource file (.resx) - call the strings per the enum's names, and then retrieve the caption from the resource strings:
var description = resx.ResourceManager.GetString(hemType.ToString());
Bottom line, I've seen worse WinForms code (and WPF for that matter), but it could be improved.