2012/4/4 Łukasz Langa <luk...@langa.pl>:
> Wiadomość napisana przez Tom Evans w dniu 4 kwi 2012, o godz. 18:40:
>
>> The class definition grates. When we say things like:
>>
>> class Gender(Choices):
>> male = Choice("male")
>>
>> That says to me that Gender.male is mutable. Ick.
>
> Thanks for your feedback. Do model and form field definitions say that as
> well? It's true that I could have block users from mutating the value but
> "we're all consenting adults here", right?
Yes, they do. If I have a model with a field named 'address', I expect
that different instances of that model may have different values for
that attribute.
If on the other hand, I have a class with an attribute 'MALE', then I
expect that value to be immutable.
>
>> There is no easy way by inspecting the code to see what choice a value
>> of 7 equates to any-more.
>
> True, the same goes for spotting errors with manual numbering when there are
> many values.
That makes no sense. If you want to see what value a choice has with
the proposed system, you have to work out precisely what ids will be
automagically assigned to a particular choice.
If you want to spot errors with the current system, you simply look
for choices with the same number. You don't have to work out what
number a choice is, it is explicitly there.
>
>> Do the choices start from 0 or do they start
>> from 1, as inferred by the examples (Why? What is wrong with 0?).
>> Repetition isn't good, but ambiguity is worse.
>
> It's a matter of aesthetics and as such it's totally subjective. I made the
> values start from 1 so that the first Choice.Group can have value 0 and not
> -1 which looks ugly.
>
>> This grouping system just seems destined for data loss/confusion. If I
>> want to split a group in two, the enums in the new group change
>> values! That is not a good approach, and was not necessary with the
>> old system.
>
> I can't see how they have to.
So, If I take your examples for groups, and annotate it with the
automagic assigned id:
> class License(Choices):
> COPYLEFT = Choices.Group(0)
> gpl_any = Choice("GPL, any") # id: 1
> gpl2 = Choice("GPL 2") # id: 2
> gpl3 = Choice("GPL 3") # id: 2
> lgpl = Choice("LGPL") # id: 3
> agpl = Choice("Affero GPL") # id: 4
>
> PUBLIC_DOMAIN = Choices.Group(100)
> bsd = Choice("BSD") # id: 101
> public_domain = Choice("Public domain") # id: 102
>
> OSS = Choices.Group(200)
> apache2 = Choice("Apache 2") # id: 201
> mozilla = Choice("MPL") # id: 202
and now I decide that I want BSD licenses as a separate group to
public domain licenses, as they are not the same thing.
> class License(Choices):
> COPYLEFT = Choices.Group(0)
> gpl_any = Choice("GPL, any") # id: 1
> gpl2 = Choice("GPL 2") # id: 2
> gpl3 = Choice("GPL 3") # id: 2
> lgpl = Choice("LGPL") # id: 3
> agpl = Choice("Affero GPL") # id: 4
>
> PUBLIC_DOMAIN = Choices.Group(100)
> public_domain = Choice("Public domain") # id: 101
>
> BSD = Choices.Group(150)
> bsd = Choice("BSD") # id: 151
>
> OSS = Choices.Group(200)
> apache2 = Choice("Apache 2") # id: 201
> mozilla = Choice("MPL") # id: 202
public_domain has changed from 102 => 101
bsd has changed from 101 => 151
These ids only change because ids in this system are handed out
automagically in order of appearance in the class, and grouping is
handled by order of appearance in the class.
>> If I had a vote, I'd be strongly -1 on
>> any proposal with this sort of grouping, it seems dangerous and wrong.
>
> Can you elaborate on what is dangerous about them?
So as above. Your counter will be "well, in those cases you need to
explicitly set an id on those choices". This relies on the person
making the changes realizing that there is magic happening, and take
that into account. That is dangerous compared to the current system,
where I would trust even a novice developer to change and move around
options.
This is because the current system is explicitly clear in what is happening.
>
>> Finally, the proposal seems to concentrate solely on choices as an
>> enum. The proposal completely ignores that choices can be almost any
>> kind of value, eg:
>>
>> MALE="m"
>> FEMALE="f"
>> UNKNOWN="?"
>> GENDER_CHOICES = (
>> (MALE, "Male"),
>> (FEMALE, "Female"),
>> (UNKNOWN, "Unknown"),
>> )
>>
>> this would be an entirely appropriate choices for a CharField.
>
> Using code in my proposal:
>
>>>> class Gender(Choices):
> ... m = Choice("male")
> ... f = Choice("female")
> ... n = Choice("not specified")
> ...
>>>> Gender(item=lambda c: (c.name, c.desc))
> [(u'm', u'male'), (u'f', u'female'), (u'n', u'not specified')]
>
So instead of MALE and FEMALE, Gender.m? No thanks.
Also, your counter-example is not exactly equivalent, and shows one
other thing that I missed - now the only valid choices are those that
can be used as python identifiers. You could not continue to have '?'
as the chosen choice for UNKNOWN.
Cheers
Tom
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
