1
\$\begingroup\$

I've always found coding UIs in Java very inefficient, and hopefully this is because of my misunderstanding. Here's a great example.

I have a UI class which is returning the String "Hours", which is effectively going to be a CSV file containing opening and closing hours of a business, Monday through Sunday. Each of these values come from individual combo boxes. So to build this String, I was curious - is there any possible way to improve it through, say, loops or some kind of recursive function?

Otherwise it's incredibly difficult to read. Any suggestions how to improve this disgusting code?

String hours = cbMondayStart.getItemAt(cbMondayStart.getSelectedIndex()) + "," +
    cbMondayFinish.getItemAt(cbMondayFinish.getSelectedIndex()) + "," +
    cbTuesdayStart.getItemAt(cbTuesdayStart.getSelectedIndex()) + "," +
    cbTuesdayStart.getItemAt(cbTuesdayStart.getSelectedIndex()) + "," +
    cbTuesdayFinish.getItemAt(cbTuesdayFinish.getSelectedIndex()) + "," +
    cbWednesdayStart.getItemAt(cbWednesdayStart.getSelectedIndex()) + "," +
    cbWednesdayFinish.getItemAt(cbWednesdayFinish.getSelectedIndex()) + "," +
    cbThursdayStart.getItemAt(cbThursdayStart.getSelectedIndex()) + "," +
    cbThursdayFinish.getItemAt(cbThursdayFinish.getSelectedIndex()) + "," +
    cbFridayStart.getItemAt(cbFridayStart.getSelectedIndex()) + "," +
    cbFridayFinish.getItemAt(cbFridayFinish.getSelectedIndex()) + "," +
    cbSaturdayStart.getItemAt(cbSaturdayStart.getSelectedIndex()) + "," +
    cbSaturdayFinish.getItemAt(cbSaturdayFinish.getSelectedIndex()) + "," +
    cbSundayStart.getItemAt(cbSundayStart.getSelectedIndex()) + "," +
    cbSundayFinish.getItemAt(cbSundayFinish.getSelectedIndex());
\$\endgroup\$
3
  • 4
    \$\begingroup\$ How about putting the references of the elements in a list when you create them, and using a loop? \$\endgroup\$
    – RealSkeptic
    Commented Aug 13, 2015 at 16:20
  • 4
    \$\begingroup\$ String hours = comboBoxes.stream().map(c -> String.valueOf(c.getSelectedIndex()).collect(Collectors.joining(","); \$\endgroup\$
    – JB Nizet
    Commented Aug 13, 2015 at 16:24
  • \$\begingroup\$ Rule 1 of programming: don't blame the compiler. Obvious parallel: don't blame the programming language. \$\endgroup\$
    – user10629
    Commented Aug 13, 2015 at 16:50

1 Answer 1

3
\$\begingroup\$

Look for repeated code and pull it out:

Stream.of(cbMondayStart, cbMondayFinish, cbTuesdayStart, cbTuesdayFinish, ...)
    .map(e -> e.getItemAt(e.getSelectedIndex()))
    .map(String::valueOf)
    .collect(Collectors.joining(","));
\$\endgroup\$
1
  • \$\begingroup\$ Works a charm, had this code in the end for anyone else looking for a finale: String hours = Stream .of(cbMondayStart, cbMondayFinish, cbTuesdayStart, cbTuesdayFinish, cbWednesdayStart, cbWednesdayFinish, cbThursdayStart, cbThursdayFinish, cbFridayStart, cbFridayFinish, cbSaturdayStart, cbSaturdayFinish, cbSundayStart, cbSundayFinish) .map(element -> element.getItemAt(element .getSelectedIndex())).map(String::valueOf) .collect(Collectors.joining(",")); \$\endgroup\$
    – Ken Reid
    Commented Aug 13, 2015 at 16:48

Your Answer

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