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
    Aug 13 '15 at 16:20
  • 4
    \$\begingroup\$ String hours = comboBoxes.stream().map(c -> String.valueOf(c.getSelectedIndex()).collect(Collectors.joining(","); \$\endgroup\$
    – JB Nizet
    Aug 13 '15 at 16:24
  • \$\begingroup\$ Rule 1 of programming: don't blame the compiler. Obvious parallel: don't blame the programming language. \$\endgroup\$
    – DavidS
    Aug 13 '15 at 16:50
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
    Aug 13 '15 at 16:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy