[Bugs] #1630 NORM: Support for mobile broadband provider database

[Sugar Labs Bugs]

#1630: Support for mobile broadband provider database
----------------------------+-----------------------------------------------
    Reporter:  aa           |          Owner:  aa         
        Type:  enhancement  |         Status:  new        
    Priority:  normal       |      Milestone:  0.90       
   Component:  sugar        |        Version:  Unspecified
    Severity:  Minor        |       Keywords:  3g r!      
Distribution:  Unspecified  |   Status_field:  New        
----------------------------+-----------------------------------------------
Changes (by erikos):

  * keywords:  3g r+ => 3g r!


Comment:

 Great that Gary is now happy with the design. Thanks for your great work
 so far. I have a few code picks:

 {{{
 try:
         84              tree = ElementTree(file=PROVIDERS_PATH)
         85              elem = tree.getroot()
         86              if elem is None or elem.get('format') !=
 PROVIDERS_FORMAT_SUPPORTED:
         87                  return False
         88              return True
         89          except IOError:
         90              return False
 }}}
 Please only put in the try/except block what you would expect an IOError
 from. Otherwise you hide errors. You can use an else block.

 {{{
 COUNTRY_CODE = locale.getdefaultlocale()[0][3:5].lower()
 }}}
 This looks dangerous. Will that always return what you expect? It is good
 to put that in a few lines, too and do error handling.

 {{{
 LANG_NS_ATTR = '{http://www.w3.org/XML/1998/namespace}lang'
 LANG = locale.getdefaultlocale()[0][:2]
 DEFAULT_NUMBER = '*99#'
 }}}
 If those are only used locally please make them private.

 {{{
 gtk.HBox.__init__(self, spacing=style.DEFAULT_SPACING*2)
 }}}
 Please use spaces. (There are a few more)

 {{{
 # Copyright (C) 2010 Andrés Ambrois
 }}}
 Character.

 In general, I am a bit worried about the error handling in
 http://bugs.sugarlabs.org/attachment/ticket/1630/0001-Add-models-for-
 detecting-and-parsing-the-providers-D.patch If you have not done so yet,
 running that code with a lot of data for testing would be good.

-- 
Ticket URL: <http://bugs.sugarlabs.org/ticket/1630#comment:12>
Sugar Labs <http://sugarlabs.org/>
Sugar Labs bug tracking system