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.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Aug 30, 2019 at 3:22
\$\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.

answered Aug 30, 2019 at 18:53
\$\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\$ Commented 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
answered Aug 30, 2019 at 18:22
\$\endgroup\$
1
  • \$\begingroup\$ I’ll be able to test this code out tomorrow, thanks for answering. \$\endgroup\$ Commented Aug 31, 2019 at 17:11

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.