Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Field in attributes changed to properties #1

Merged
merged 1 commit into from
May 12, 2012
Merged

Field in attributes changed to properties #1

merged 1 commit into from
May 12, 2012

Conversation

gimmemoore
Copy link
Collaborator

I have change the use of public fields in OptionAttributes and ValueList to make use of property istead.

gsscoder added a commit that referenced this pull request May 12, 2012
Field in attributes changed to properties (merged from gimmemoore).
@gsscoder gsscoder merged commit 939eae0 into gsscoder:master May 12, 2012
@gsscoder
Copy link
Owner

Ouch!!! Just one problem with your changes: DefaultValue works correctly only under Visual Studio Designer.

I'm agree to evolve the API for target only property (I've accepted your pull request!), but with this design decision everything different from null, false and first item of enum must be initialized in Options class costructor.

Regards,
Giacomo

@gimmemoore
Copy link
Collaborator Author

It could also be achieve with standard properties, where a class variable hold the value of the property, and therefore the default value. This would ease the migration to Properties for current user, as you should suggest them to convert all their member with OptionAttributes, to properties using the existing fields, and of course they'll have to move the OptionAttribute to the property.

@gsscoder
Copy link
Owner

It's a minor change in favor of more object oriented design. Initializing defaults in constructor enforce a cleaner design. For now I'll leave everything this way... It's ok!

But I liked the System.ComponentModel.DefaultAttribute as idea... I'm wondering to support it as an OptionAttribute property. What do you think?

@gimmemoore
Copy link
Collaborator Author

I think that would be great, easy to understand and use, this sould definitvely be done as an OptionAttribute property!

gsscoder pushed a commit that referenced this pull request Aug 10, 2015
Data type was missing in an example in README
nemec pushed a commit that referenced this pull request Jan 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants