3
\$\begingroup\$

I'm wondering what I could do to improve the function that handles initializing the UI. I think it looks disgusting. Any ideas on what I could do besides splitting the code up into multiple functions to make it look better?

def init_ui(window, root_dir):
    window.title('ACFA Save Editor')
    tk.Label(window, text='Pilot name: ').grid(row=0)
    tk.Label(window, text='AC name: ').grid(row=1)
    tk.Label(window, text='COAM (money): ').grid(row=2)
    tk.Label(window, text='FRS Memory: ').grid(row=3)
    e_p_name = tk.Entry(window)
    e_p_name.grid(row=0, column=1)
    e_ac_name = tk.Entry(window)
    e_ac_name.grid(row=1, column=1)
    e_coam = tk.Entry(window)
    e_coam.grid(row=2, column=1)
    e_frs_memory = tk.Entry(window)
    e_frs_memory.grid(row=3, column=1)
    messagebox.showinfo('Open Save Folder', 'Ex: GAMEDATXXXX')
    directory = askdirectory(initialdir=root_dir)
    APGD = directory + '/APGD.dat'
    GAMEDAT = directory + '/' + directory.split('/')[-1] + '_CONTENT'
    p, a, c, f = read_data(APGD)
    e_p_name.insert(0, p)
    e_ac_name.insert(0, a)
    e_coam.insert(0, c)
    e_frs_memory.insert(0, f)
    b_apply = tk.Button(window, text='Apply', width=10, command=lambda: 
        apply(e_p_name.get(), e_ac_name.get(), e_coam.get(), 
        e_frs_memory.get(), 
        APGD, GAMEDAT)).grid(row=4, column=0)
    b_quit = tk.Button(window, text='Quit', width=10, 
        command=window.destroy).grid(row=4, column=1)
    return window

The rest of the code can be found here.

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

Loops are your friend. Much of your function can be replaced with:


    for row, (name, data) in enumerate(zip((
        'Pilot name',
        'AC name',
        'COAM (money)',
        'FRS Memory'
    ), read_data(APGD))):
        tk.Label(window, text=f'{name}: ').grid(row=row)
        e = tk.Entry(window)
        e.grid(row=row, column=1)
        e.insert(0, data)

I'm unable to test this, so YMMV.

\$\endgroup\$
1
  • \$\begingroup\$ I’ll have some time to test this code out tomorrow and then I’ll go from there, thanks for your answer. \$\endgroup\$ Aug 31, 2019 at 17:11
2
\$\begingroup\$

It depends what direction you want to go with your program, but my suggestion would be to make small factories for components you use the most often. Here it seems you are building a common UI feature: a form with several fields. Since you only have a single form you don't really benefit from abstracting the form for now, except for clarity, but you benefit from abstracting the fields as you use 4 of them.

So this code separates better generic UI components and data although it's not perfect; but UI code rarely is. Since you usually only have a handfew windows in the average app it's often OK to hardcode some values and components. It's a matter of making compromises between genericity, features, and verbosity.

(Disclaimer: I haven't tested it, please let me know if it needs fixing)

class FormField:
    def __init__(self, window, row, label, placeholder=None):
        tk.Label(window, text=label).grid(row=row)
        self.entry = tk.Entry(window)
        self.entry.grid(row, column=1)
        if placeholder:
            self.entry.insert(0, placeholder)

    def get(self):
        return self.entry.get()

class Form:
    def __init__ (self, window, row, field_keywords, callback, button_text):
        self.fields = []
        for i, field_keyword in enumerate(field_keywords):
             self.fields.append(window, row=row + i, **field_keyword)
        self.callback = callback
        # This '10' is hardcoded, but you could compute it from button_text length
        b_apply = tk.Button(window, text=button_text, width=10, command=self.submit))

    def submit(self):
        self.callback(*[field.get() for field in self.fields])

def init_ui(window, root_dir):
    # Below is code that is tricky to make any more generic and can be left as is
    # Making UI abstractions for single use components can be complexity you don't need
    window.title('ACFA Save Editor')
    messagebox.showinfo('Open Save Folder', 'Ex: GAMEDATXXXX')
    directory = askdirectory(initialdir=root_dir)
    APGD = directory + '/APGD.dat'
    GAMEDAT = directory + '/' + directory.split('/')[-1] + '_CONTENT'
    p, a, c, f = read_data(APGD)
    fields_kws = [{'label':'Pilot name: ', 'placeholder':p},
                  {'label':'AC name: ', 'placeholder':a},
                  {'label':'COAM (money): ', 'placeholder':c},
                  {'label':'FRS Memory: ', 'placeholder':f}]
    ui_form = Form(window, 1, field_kws, lambda p, a, c, f: apply(p, a, c, f, APGD, GAMEDAT), 'Apply')

    b_quit = tk.Button(window, text='Quit', width=10, 
        command=window.destroy).grid(row=4, column=1)
    return window

\$\endgroup\$
1
  • \$\begingroup\$ I’ll be able to test this code out tomorrow, thanks for answering. \$\endgroup\$ Aug 31, 2019 at 17:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.