I would like to delete the duplicate source code below.
public static List<Map<String, Object>> getTransmissionTypeList() {
List<Map<String, Object>> transmissionList = new ArrayList<>();
for(EstimateTransmissionType transmissionType : EstimateTransmissionType.values()) {
Map<String, Object> transmission = new HashMap<>();
transmission.put("code", transmissionType.getCode());
transmission.put("value", transmissionType.getValue());
transmission.put("name", transmissionType.getName());
transmissionList.add(transmission);
}
return transmissionList;
}
public static List<Map<String, Object>> getFuelTypeList() {
List<Map<String, Object>> fuelList = new ArrayList<>();
for(EstimateFuelType fuelType : EstimateFuelType.values()) {
Map<String, Object> transmission = new HashMap<>();
transmission.put("code", fuelType.getCode());
transmission.put("value", fuelType.getValue());
transmission.put("name", fuelType.getName());
fuelList.add(transmission);
}
return fuelList;
}
For the EstimateTransmissionType
and EstimateFuelType
, those two enums are totally same structure inside.
public enum EstimateTransmissionType {
AUTO("오토(A/T)", "A001", "001"),
MANUAL("수동(M/T)", "A002", "002"),
ETC("기타", "A999", "999");
final String name;
final String value;
final String code;
EstimateTransmissionType(String name, String value, String code) {
this.name = name;
this.value = value;
this.code = code;
}
public String getName() {
return name;
}
public String getValue() {
return value;
}
public String getCode() {
return code;
}
}
public enum EstimateFuelType {
GASOLINE("가솔린", "A001", "001"),
DIESEL("디젤", "A002", "002"),
LPG_GENERAL("LPG(일반인 구입)", "A003", "003"),
LPG_ALL("LPG", "A004", "004"),
HYBRID("가솔린 전기", "A005", "005"),
ELECTRONIC("전기", "A009", "009"),
ETC("기타", "A999", "999");
final String name;
final String value;
final String code;
EstimateFuelType(String name, String value, String code) {
this.name = name;
this.value = value;
this.code = code;
}
public String getName() {
return name;
}
public String getValue() {
return value;
}
public String getCode() {
return code;
}
}
I really want to improve my source code quality, but I have no idea where to start this. Could somebody give me a good inspiration or clear answer?
CodePudding user response:
This answer is related and helpful -- https://stackoverflow.com/a/70596442/10118965
But here is a more directed answer. In short, using interfaces, enums, and generics allows you to make this much easier to consolidate.
First, create an interface that has all of the relevant functions. I called mine ParentInterface
. In this case, you needed ::getName
, ::getValue
, and ::getCode
.
Next, you need to create a method inside that interface that returns the values of the enum. This is a more complicated step because it requires you to take a pretty big step into the generics world, combined with enums.
In short, if we look at the JavaDoc for enums, we see the following.
https://docs.oracle.com/en/java/javase/19/docs/api/java.base/java/lang/Enum.html
Class Enum<E extends Enum<E>>
So in short, if we want to say that some type E
is an enum, then we must say something like this.
E extends Enum<E>
Ok, now we know how to say that the parameterized type must be an enum. The reason why we did this is because we actually wanted to get the values of the enum. We can't do that without first making sure that we have an enum.
But now that we do, we can write a method sort of like this.
public static <E extends Enum<E>> List<E> values(final Class<E> clazz)
{
return Arrays.asList(clazz.getEnumConstants());
}
This method says that the clazz
that we receive as a paramter must be a type of enum. Once it proves that, then we can safely call the Class::getEnumConstants
method and be sure that we will get what we expect.
So, now that we have wrote our static method, next we put a parameterized type on the interface. Specifically, we want to make sure that this interface only refers to enums.
We add the following parameterized type to the interface.
<E extends Enum<E>>
And then finally, we modify our static method just slightly to make sure that it is only used to generate enum values for enums that implement our interface. Maybe you don't want that restriction, but in case you do, here is what you would add.
& ParentInterface<E>
And with that, we are done. Here is what our interface should roughly look like.
public interface ParentInterface<E extends Enum<E>> // pick a better name
{
String getName();
String getValue();
String getCode();
public static <E extends Enum<E> & ParentInterface<E>> List<E> values(final Class<E> clazz)
{
return Arrays.asList(clazz.getEnumConstants());
}
}
Next, we need to make sure all of our enums implement this new interface. That is pretty easy to do.
public enum EstimateFuelType implements ParentInterface<EstimateFuelType>
public enum EstimateTransmissionType implements ParentInterface<EstimateTransmissionType>
The actual internals of the enum shouldn't have to change. All you are doing is changing the outside.
And then finally, let us rework our method to handle this brand new interface and our reworked enums.
Let's start by building off of our ::getFuelTypeList
method. That means that we start with a duplicate, but renamed.
public static List<Map<String, Object>> getGenericTypeList() {
List<Map<String, Object>> fuelList = new ArrayList<>();
for(EstimateFuelType fuelType : EstimateFuelType.values()) {
Map<String, Object> transmission = new HashMap<>();
transmission.put("code", fuelType.getCode());
transmission.put("value", fuelType.getValue());
transmission.put("name", fuelType.getName());
fuelList.add(transmission);
}
return fuelList;
}
Other than the method name, this method is exactly the same as ::getFuelTypeList
.
Let's start by changing the method signature to include the generics that we made. More specifically, let's introduce this method to the idea of our interface.
public static <E extends ParentInterface<E>> List<Map<String, Object>> getGenericTypeList()
However, if we try to do just this, the compiler will complain at us, saying that if we want E
to extend our interface, it must also extend Enum<E>
too. Otherwise, it will break the rule of our interface, which says that "Anything that extends me must have an enum in the type parameter that also extends me". We made this rule earlier on when putting the type parameter on our interface.
So, to get past this, we must specify that our type parameter also is an enum. That is easy enough to change.
public static <E extends Enum<E> & ParentInterface<E>> List<Map<String, Object>> getGenericTypeList()
Now, the compiler is happy.
So, because of this change, our method is now aware of a type called E
. This type is a subtype of an enum, and is also a subtype of the interface that we made.
Now that the method is aware of our type, let's actually put it to use. Here is what we have thus far.
public static <E extends Enum<E> & ParentInterface<E>> List<Map<String, Object>> getGenericTypeList() {
List<Map<String, Object>> fuelList = new ArrayList<>();
for(EstimateFuelType fuelType : EstimateFuelType.values()) {
Map<String, Object> transmission = new HashMap<>();
transmission.put("code", fuelType.getCode());
transmission.put("value", fuelType.getValue());
transmission.put("name", fuelType.getName());
fuelList.add(transmission);
}
return fuelList;
}
Obviously, everything is still hardcoded to use EstimateFuelType
. So let's start by changing that.
public static <E extends Enum<E> & ParentInterface<E>> List<Map<String, Object>> getGenericTypeList() {
List<Map<String, Object>> fuelList = new ArrayList<>();
for(E fuelType : E.values()) {
Map<String, Object> transmission = new HashMap<>();
transmission.put("code", fuelType.getCode());
transmission.put("value", fuelType.getValue());
transmission.put("name", fuelType.getName());
fuelList.add(transmission);
}
return fuelList;
}
This almost works, but the compiler gets upset and says that it doesn't know where to find the E::values
method. Annoyingly enough, Java does not permit us to override static methods, so this solution had to get needlessly complicated. Luckily, we already did the complicated part earlier. Instead of trying to use the unknown E::values
method, why don't we use the static method we made earlier?
However, in order to use that method, we need to provide a Class<E>
. Simple enough, let's also modify our method to take in a Class<E>
. And let's also replace that unknown method that just threw an error.
public static <E extends Enum<E> & ParentInterface<E>> List<Map<String, Object>> getGenericTypeList(final Class<E> clazz) {
List<Map<String, Object>> fuelList = new ArrayList<>();
for(E fuelType : ParentInterface.values(clazz)) {
Map<String, Object> transmission = new HashMap<>();
transmission.put("code", fuelType.getCode());
transmission.put("value", fuelType.getValue());
transmission.put("name", fuelType.getName());
fuelList.add(transmission);
}
return fuelList;
}
And then that should be it. You can now delete the unnecessary methods that only work for the individual enums. Here is the complete solution, with some refactoring, renaming, and general clean up.
import java.util.Arrays;
import java.util.List;
import java.util.Map;
public class SOQ_20230108
{
public static void main(String[] args)
{
System.out.println(getParentTypeList(EstimateFuelType.class));
System.out.println(getParentTypeList(EstimateTransmissionType.class));
}
public interface ParentInterface<E extends Enum<E>> // pick a better name
{
String getName();
String getValue();
String getCode();
public static <E extends Enum<E>> List<E> values(final Class<E> clazz)
{
return Arrays.asList(clazz.getEnumConstants());
}
}
public enum EstimateFuelType implements ParentInterface<EstimateFuelType>
{
GASOLINE("가솔린", "A001", "001"),
DIESEL("디젤", "A002", "002"),
LPG_GENERAL("LPG(일반인 구입)", "A003", "003"),
LPG_ALL("LPG", "A004", "004"),
HYBRID("가솔린 전기", "A005", "005"),
ELECTRONIC("전기", "A009", "009"),
ETC("기타", "A999", "999");
final String name;
final String value;
final String code;
EstimateFuelType(String name, String value, String code)
{
this.name = name;
this.value = value;
this.code = code;
}
public String getName()
{
return name;
}
public String getValue()
{
return value;
}
public String getCode()
{
return code;
}
}
public enum EstimateTransmissionType implements ParentInterface<EstimateTransmissionType>
{
AUTO("오토(A/T)", "A001", "001"),
MANUAL("수동(M/T)", "A002", "002"),
ETC("기타", "A999", "999");
final String name;
final String value;
final String code;
EstimateTransmissionType(String name, String value, String code)
{
this.name = name;
this.value = value;
this.code = code;
}
public String getName()
{
return name;
}
public String getValue()
{
return value;
}
public String getCode()
{
return code;
}
}
public static <E extends Enum<E> & ParentInterface<E>> List<Map<String, Object>> getParentTypeList(Class<E> clazz) {
final List<Map<String, Object>> typeList = new ArrayList<>();
for(final E type : ParentInterface.values(clazz)) {
final Map<String, Object> typeMap = new HashMap<>();
typeMap.put("code", type.getCode());
typeMap.put("value", type.getValue());
typeMap.put("name", type.getName());
typeList.add(typeMap);
}
return typeList;
}
}
This complete example has a main method, so you can run it yourself and see that it works.
Finally, if you decide to make this method work for other enums that you add into the future, it is very easy to do. Simply ensure that each enum you add implements your interface, and the parameterized type for that interface should be the enum name itself. If that doesn't make sense, just follow the same format as the other enums and it should work out fine.
CodePudding user response:
The new old wisdom to prefer composition over inheritance seems to apply here. We can do all the common stuff in a delegate class:
import java.util.Arrays;
import java.util.Map;
import java.util.HashMap;
import java.util.List;
import java.util.stream.Collectors;
public class EnumDemo {
interface AttributeProvider {
EstimateAttributes getAttributes();
}
public enum EstimateTransmissionType implements AttributeProvider {
AUTO("오토(A/T)", "A001", "001"),
MANUAL("수동(M/T)", "A002", "002"),
ETC("기타", "A999", "999");
private final final EstimateAttributes attributes;
EstimateTransmissionType(String name, String value, String code) {
this.attributes = new EstimateAttributes(name, value, code);
}
@Override
public EstimateAttributes getAttributes() {
return attributes;
}
}
public enum EstimateFuelType implements AttributeProvider {
GASOLINE("가솔린", "A001", "001"),
DIESEL("디젤", "A002", "002"),
LPG_GENERAL("LPG(일반인 구입)", "A003", "003"),
LPG_ALL("LPG", "A004", "004"),
HYBRID("가솔린 전기", "A005", "005"),
ELECTRONIC("전기", "A009", "009"),
ETC("기타", "A999", "999");
private final EstimateAttributes attributes;
EstimateFuelType(String name, String value, String code) {
attributes = new EstimateAttributes(name, value, code);
}
@Override
public EstimateAttributes getAttributes() {
return attributes;
}
}
static class EstimateAttributes {
private final String name;
private final String value;
private final String code;
public EstimateAttributes(String name, String value, String code) {
this.name = name;
this.value = value;
this.code = code;
}
public Map<String, Object> asMap() {
Map<String, Object> map = new HashMap<>();
map.put("name", name);
map.put("value", value);
map.put("code", code);
return map;
}
// Add getters if needed.
}
public static <E extends Enum<E> & AttributeProvider>
List<Map<String, Object>> getAttributeMaps(Class<E> enumClass) {
return Arrays.stream(enumClass.getEnumConstants())
.map(enumValue -> enumValue.getAttributes().asMap())
.collect(Collectors.toList());
}
}
Now we can say EnumDemo.getAttributeMaps(EstimateTransmissionType.class)
and EnumDemo.getAttributeMaps(EstimateFuelType.class)
.