Reputation: 2506
I have two enums which are related.
Enum1:
public enum HttpMethodName
{
GET, POST, PUT, DELETE;
}
Enum2:
public enum ProtocolOperation {
CREATE(1), RETRIEVE(2), UPDATE(3), DELETE(4), NOTIFY(5);
private BigInteger operationId;
public BigInteger getOperationId() {
return operationId;
}
private ProtocolOperation(int operationId) {
this.operationId = BigInteger.valueOf(operationId);
}
}
The enums values have mapping as:
Create--> POST
Retrieve--> GET
Update--> PUT
Delete--> DELETE
Notify---> POST
So there is one to one mapping between values except only for POST
case which can have two values either Create
or Notify
based on a condition.
I was thinking of keeping the mapping as List
:
public enum HttpMethodName
{
POST(new List(ProtocolOperation.CREATE, ProtocolOperation.NOTIFY)) ,GET ( new List(ProtocolOperation.RETRIEVE) ) ,
PUT (new List(ProtocolOperation.UPDATE) ,DELETE(new List(ProtocolOperation.DELETE) ;
List<ProtocolOperation > ops;
HttpMethodName (List<ProtocolOperation> ops)
{
this.ops = ops;
}
}
Is there a better approach??
EDIT:
I will want mapping both ways from HttpMethodName <----> ProtocolOperation
Upvotes: 9
Views: 2952
Reputation: 159114
Create the mapping from ProtocolOperation
to HttpMethodName
like you showed with the arrows, so they are simple references, not lists.
The reverse mapping can then be a static method that iterates the 5 enum values for matches. There are only 5 values, so a sequential search is adequately fast, unless it's something you do in a very tight loop, and that's unlikely in this case. Besides, you shouldn't code for performance until a profiler says you have a problem.
You follow the mappings as follows:
// From ProtocolOperation to HttpMethodName
HttpMethodName method = ProtocolOperation.CREATE.getHttpMethodName();
// From HttpMethodName to ProtocolOperation
ProtocolOperation op = ProtocolOperation.forMethod(HttpMethodName.GET);
Implementation:
public enum HttpMethodName
{
GET, POST, PUT, DELETE;
}
public enum ProtocolOperation {
CREATE (1, HttpMethodName.POST),
RETRIEVE(2, HttpMethodName.GET),
UPDATE (3, HttpMethodName.PUT),
DELETE (4, HttpMethodName.DELETE),
NOTIFY (5, HttpMethodName.POST);
private final int operationId;
private final HttpMethodName httpMethodName;
private ProtocolOperation(int operationId, HttpMethodName httpMethodName) {
this.operationId = operationId;
this.httpMethodName = httpMethodName;
}
public int getOperationId() {
return this.operationId;
}
public HttpMethodName getHttpMethodName() {
return this.httpMethodName;
}
public static List<ProtocolOperation> forMethod(HttpMethodName httpMethodName) {
List<ProtocolOperation> ops = new ArrayList<>();
for (ProtocolOperation op : values())
if (op.httpMethodName == httpMethodName)
ops.add(op);
return ops;
}
}
Upvotes: 4
Reputation: 10882
Why do you find your current approach unsatisfactory?
Without the knowledge of your concerns I can only suggest to remove boilerplate:
import static ProtocolOperation.*;
public enum HttpMethodName {
GET(RETRIEVE),
POST(CREATE, NOTIFY),
PUT(UPDATE),
DELETE(ProtocolOperation.DELETE);
final List<ProtocolOperation> ops;
HttpMethodName(ProtocolOperation... ops) {
this.ops = Collections.unmodifiableList(Arrays.asList(ops));
}
}
If you want to have mapping in both ways, it makes sense to hardcode mapping from ProtocolOperation
to HttpMethodName
first (as it doesn't require list) and create a simple search method in MttpMethodName
:
public enum ProtocolOperation {
CREATE(1, HttpMethodName.POST),
RETRIEVE(2, HttpMethodName.GET),
UPDATE(3, HttpMethodName.PUT),
DELETE(4, HttpMethodName.DELETE),
NOTIFY(5, HttpMethodName.POST);
private BigInteger operationId;
private HttpMethodName methodName;
public BigInteger getOperationId() {
return operationId;
}
public HttpMethodName getMethodName() {
return methodName;
}
private ProtocolOperation(int operationId, HttpMethodName httpMethodName) {
this.methodName = httpMethodName;
this.operationId = BigInteger.valueOf(operationId);
}
}
public enum HttpMethodName {
GET,
POST,
PUT,
DELETE;
List<ProtocolOperation> getProtocolOperations() {
List<ProtocolOperation> ops = new ArrayList<ProtocolOperation>(2);
for (ProtocolOperation op : ProtocolOperation.values()) {
if (op.getMethodName() == this) {
ops.add(op);
}
}
return ops;
}
}
As you have constant and small number of values, you don't need to create final static map in HttpMethodName
to provide the backwards mapping, linear search is fast enough for your case.
Upvotes: 8
Reputation: 11620
I would go with separating logic from enum. Your approach is a tight coupling, less elastic. With such library with transform metod you are more flexible
private static final Map<HttpMethodName , ProtocolOperation> mapping = new HashMap<>();
static {
mapping .put(Create, POST);
// rest of methods
}
And here the body of your mapping method (I am using Jdk8):
public static Optional<HttpMethodName > map(ProtocolOperation op){
if (!mapping.containsKey(op)){
return Optional.empty();
}
return Optional.of(mapping.get(op));
}
In short: enum should not have any business logic associated with them, for mapping and transformation there should be used external methods (utils).
UPDATE: As @Andreas has correctly pointed, in this particular example, where mapping is fixed, and should not be extended, you can stay with his way of initializing:
HttpMethodName(ProtocolOperation... ops) {
this.ops = Collections.unmodifiableList(Arrays.asList(ops));
}
Upvotes: 1