5
\$\begingroup\$

I have following Python code:

def get_subject_from_stream_id_and_subject_id(stream_id, subject_id):
    #(stream_id, subject_id): ("subject_name")
    return {
        (1, 1): "Accounts",
        (1, 2): "English",
        (1, 3): "Organization of Commerce",
        (2, 1): "Physics",
        (2, 2): "English",
        (2, 3): "Biology"
    }.get((stream_id, subject_id), "None")

In this code, I want to get subject name from the integer pair combination i.e. stream_id, subject_id e.g. (1, 2) is for English. It was implemented using a Python tuple.

I want to implement the same piece of code in Java.

Could someone write this in a better way in Java?

public String getSubjectFromStreamIdAndSubjectId(int streamId, int subjectId) {
  switch (streamId) {
    case 1:
        switch (subjectId) {
        case 1:
            return "Accounts";
        case 2:
            return "English";
        case 3:
            return "Organization of Commerce";
        default:
            return null;
        }

    case 2:
        switch (subjectId) {
        case 1:
            return "Physics";
        case 2:
            return "English";
        case 3:
            return "Biology";
        default:
            return null;
        }
    default:
        return null;
   }
}
\$\endgroup\$
4
  • \$\begingroup\$ That doesn't look like syntactically valid Java code. \$\endgroup\$ Feb 25, 2019 at 12:07
  • \$\begingroup\$ It's lacking method parameter types. Otherwise it compiles just great. \$\endgroup\$ Feb 25, 2019 at 12:31
  • \$\begingroup\$ @200_success I updated it \$\endgroup\$ Feb 25, 2019 at 13:18
  • \$\begingroup\$ This looks like a fairly simple 2D array lookup. Just subtract 1 from the indices, and check they're in bounds. Any solution involving a dictionary is inefficient - both in terms of performance and code complexity. \$\endgroup\$
    – bace1000
    Feb 25, 2019 at 20:47

3 Answers 3

4
\$\begingroup\$

Java equivalent of the above Python code goes more like this:

private static final Map<List<Integer>, String> SUBJECT_MAP = createSubjectMap();

private static Map<List<Integer>, String> createSubjectMap() {
    Map<List<Integer>, String> map = new HashMap<>();
    map.put(asList(1, 1), "Accounts");
    map.put(asList(1, 2), "English");
    map.put(asList(1, 3), "Organization of Commerce");
    map.put(asList(2, 1), "Physics");
    map.put(asList(2, 2), "English");
    map.put(asList(2, 3), "Biology");
    return map;
}

public static String getSubjectFromStreamIdAndSubjectId(int streamId, int subjectId) {
    return SUBJECT_MAP.get(asList(streamId, subjectId));
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Might be better to use a custom SubjectIdentifier class since a List seems rather inefficient. \$\endgroup\$
    – rath
    Feb 25, 2019 at 16:21
  • \$\begingroup\$ @rath I don't know about efficiency, but it would certainly be more idiomatic java. OP seems to be new to java, and I wanted to show him that Lists support equals and hashcode thus can be used as keys in maps and sets. \$\endgroup\$ Feb 26, 2019 at 6:54
  • \$\begingroup\$ Why not just concatenate the integers into a String? map.put("1.1", "Accounts") ... map.get(streamId + "." + subjectId)? I'm aware it's not Pure OOP, but it's "only an internal data structure." \$\endgroup\$ Feb 26, 2019 at 8:14
4
\$\begingroup\$

Neither is good, but surely we can all agree that the Java example is way uglier. The biggest problem, however, is that both examples hard code data into code.

Separate data, i.e. stream and subject IDs and their titles into a data class and in that class implement code that accesses the data structure without detailed knowledge about the actual data. Responsibility of setting up the data structure to resemble your stream and subject numbering is left to a separate component (load it from file or set up in static code).

Whether the data class is a recursive structure or just a wrapper for a HashMap depends on the complexity of your data.

\$\endgroup\$
0
3
\$\begingroup\$

When something is difficult in Java it's nearly always because there is a better way to do it (in Java, it generally means you are missing a class). In your case, it looks like you need a "Subject" class.

Let's say you had a "Subject" class, how would it be implemented? Here's one way (Not my favorite)

enum Subject {
    Accounts(1,1),
    English(1,2),
    …;
   public static Subject getSubject(streamId, subjectId) {
       // Iterate over Subject.values and return one that matches
   }

This means that your call becomes simple:

assertEquals(Subjects.Accounts, Subject.getSubject(1,1));

I don't completely love this because it's still hard-coded. I would personally make the Subjects class a full class and load it from either a database or a data file, but it should still have a getSubject() static.

Just in case you think there is some kind of performance issue caused by looping instead of a switch, note that A) java is 10x faster than python to start and B) premature optimization is the root of all evil (well, lots of evil)

\$\endgroup\$

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.