[pve-devel] [PATCH manager 2/4] kvm_ostype: move to store-like format

[Thomas Lamprecht]

On 08/24/2017 11:24 AM, Dominik Csapak wrote:
> On 08/22/2017 11:57 AM, Thomas Lamprecht wrote:
>> [snip]
>>       get_health_icon: function(state, circle) {
>> @@ -124,15 +132,27 @@ Ext.define('PVE.Utils', { utilities: {
>>       return state;
>>       },
>> -    render_kvm_ostype: function (value) {
>> -    if (!value) {
>> -        return gettext('Other OS types');
>> +    get_kvm_osinfo: function(value) {
>> +    var info = { base: 'Other' }; // default
>> +    if (value) {
>> +        Ext.each(Object.keys(PVE.Utils.kvm_ostypes), function(k) {
>> +        Ext.each(PVE.Utils.kvm_ostypes[k], function(e) {
>> +            if (e.val === value) {
>> +            info = { desc: e.desc, base: k };
>> +            }
> 
> here you could do a return false; to cancel the loop when you found the value
> you could also do this with the outer loop, but this would involve checking if you already found your value, which is maybe not that of an optimization
> 

Yes I had this in my internal v1, but I didn't liked the whole checking
and as the data structure is really small and will stay small as you noted
yourself below, I just threw it away.

> but then again my whole comment is not much of an optimization when
> going through a list with 4 elements :P
> 
> i am not opposed to use it as it is, because it will not be slow
> and it is still readable
> 

But yes, I find it ugly too (the nested ext.each), but I really really like the
used data structure as I can directly use it as store for the combobox and do not
need to overwrite setValue stuff, so if no better one which does this too arises
I would opt for this one.

much thanks for the review!