New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support enumeration types in Umple #1008

Closed
vahdat-ab opened this Issue Mar 1, 2017 · 17 comments

Comments

Projects
None yet
3 participants
@vahdat-ab
Member

vahdat-ab commented Mar 1, 2017

Enumeration types are modeling elements and part of many class diagrams,
As long as I 'm aware there is no such implementation for it. Even we cannot see it in the graphical representation of my class diagram.
I'm also aware that someone might want to define an enumeration like below in Umple

class A{
  status{married, single, divorced}
}

However, I argue that it's not correct.
Generated code:

public enum Status { married, single, divorced }
private Status status;

Umple detects that one as a state machine (you can see that a state machine diagram is generated for it). A variable with the same name is generated in the java code and then the same name becomes camelcase and is considered as the name of enumeration.
These issues must be fixed. The type must be exactly what designer defines. If designers want to use enumerations, they can use it as normal types for attributes.
Plus, The graphical representation (class diagram) should be improved to present it.

@TimLethbridge

This comment has been minimized.

Member

TimLethbridge commented Mar 2, 2017

We decided long, long ago that logically a state machine is an attribute with fixed values (each of its states), so logically an enumeration is too. The key difference is that a state machine changes values by way of events. There are examples in existing systems so we can't change the generated code now.

If you want to do something different, please explain the generated code you would prefer. Maybe we can add a stereotype.

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Mar 2, 2017

@TimLethbridge A question: how we can satisfy this simple general model in Umple based on that philosophy.

enum Course { eng, csi}
class A{
    Course course1;
}

class B{
    Course course2;
}

It doesn't matter where we define an enumeration. My issue is how to deal with reusing it.
Furthermore, sometimes I just need to define it without creating an attribute, but it looks like impossible in Umple. It is forced to have an attribute with any enumeration.

@TimLethbridge

This comment has been minimized.

Member

TimLethbridge commented Mar 2, 2017

Eight then. So use the keyword enum as a stereotype to generate this. Leave the existing as it is.

@jblang94

This comment has been minimized.

Contributor

jblang94 commented Apr 4, 2017

@vahdat-ab

I've added the enumeration definition to umple_classes.grammar as follows:
enumerationDefinition : enum [name] { [enumValue] ( , [enumValue] )* }
and I've | [[enumerationDefinition]] onto classContent .

However, the parser is still parsing enumerations such as
enum status { FullTime, PartTime, MidTime } as malformedStatemachine2.

The rule for malformedStatemachine2 is defined in umple_state_machines.grammar as follows:

malformedStatemachine2 : [!stuff:[^\\{\\}\\OPEN_ROUND_BRACKET\\CLOSE_ROUND_BRACKET]*]  ( [!stuff:[^\\{\\}\\OPEN_ROUND_BRACKET\\CLOSE_ROUND_BRACKET]*] )* { [**innerstuff] }

I've tried modifying this definition, but have not been able to get the parsing to work. I've confirmed that enumerationDefinition is being read from umple_classes.grammar in readGrammarFiles() from GrammarParsing_Code.ump (generated Java file is RuleBasedParser.java).

I don't have much experience with defining parsing rules so I am wondering what is the best way to modify malformedStateMachine2?

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Apr 5, 2017

In order to fix this, make sure the rule [[enumerationDefinition]] comes before the rule exception
like this: classContent- : ... | [[enumerationDefinition]] | ; | [[exception]] | [[extraCode]]

@jblang94

This comment has been minimized.

Contributor

jblang94 commented Apr 6, 2017

@vahdat-ab enumerations are now implemented for Java generation! Couple of questions:

  1. Would you like me to create an entry for enumerations in the Umple User Manual?
  2. Do I also have to implement this in PHP?
@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Apr 6, 2017

Great. Yes please. we need to have a manual part as well.
You don't need to implement it in PHP, but you need to create an issue for it. You need to indicate that this feature needs to be implemented in other languages as well. Furthermore, create an issue that says we don't have it in our graphical representation.

I wonder if your solution satisfies the following examples:

enum Status { married, single, divorced }
class C1{
  Status status;
}
class C2{
  Status status;
}

In the generated code, we can simply repeat the definition of enum for each class.

and

class C1{
  enum Status { married, single, divorced }
  Status status;
}
@jblang94

This comment has been minimized.

Contributor

jblang94 commented Apr 6, 2017

@vahdat-ab great. I currently have it implemented per class, because I believe that's what you told me to go ahead with in our meeting a few days ago. So the second example works, but I would need to update my solution to get it to work with your first example.

I think that I would need to do the following:

  1. | [[enumerationDefinition]] onto entity in umple_core.grammar
  2. Add 1 -> * UmpleEnumeration to UmpleModel in Umple.ump
  3. Update UmpleInternalParser, and JavaCodeGenerator to handle the case when the enumerations are defined at the top-level of an UmpleModel.
@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Apr 6, 2017

because I believe that's what you told me to go ahead with in our meeting a few days ag

Yup, but would be great to have both scenarios.

@jblang94

This comment has been minimized.

Contributor

jblang94 commented Apr 6, 2017

@vahdat-ab I'll do my best to make this addition!

@jblang94

This comment has been minimized.

Contributor

jblang94 commented Apr 7, 2017

Vahdat Feedback 1 from Initial PR

Status: FIXED

Cool. Thanks.
Please consider the following example:

enum Month {january, february, March}
enum Fruit {apple, orange}
class X1{
  Month month;
}
class X2{
   Fruit t;
}

When the code is generated, I see that both enumeration types are defined in both classes. However, this should not happen because class X1 just needs the enumeration Month and class X2 just needs the enumeration Fruit.
Another example is this


enum Month {january, february, March}
enum Fruit {apple, orange}
class X{}
class X2{}

The generator still puts the enumeration in these classes.

The solution

Simply check if the enumeration has been used for attribute types, parameters of methods, or parameters of events of state machines. If it's been used once, then put the definition of enumeration inside the class.

For sure, this is going to be valid just for model-level enumerations

@jblang94

This comment has been minimized.

Contributor

jblang94 commented Apr 7, 2017

Vahdat Feedback 2 from Initial PR

Status: FIXED

Good, thanks.
You also need to make sure the names are unique. Please consider the following example:

enum Month {x,y,z}
enum Month {o,p,q}
class A{
  Month m;
  Month p;
}

The Umple compiler easily passes this. It must be detected and a proper error must be raised.
Another example

enum Month {x,y,z}
class A{
  enum Month {o,p,q}
  Month m;
  Month p;
}

In this case, there is no error because the local version has a priority over the model-level enumeration. You need to make sure you don't generate the definition for enum Month {x,y,z} in class A.
Make sure you also document the error.

@jblang94

This comment has been minimized.

Contributor

jblang94 commented Apr 7, 2017

Vahdat Feedback 3 from Initial PR

Status: NOT fixed

Great.
Just a note: when you resolve any kind of problem, you need to consider all possible cases:
Check this example:

enum X{x}
enum Y{y}
class X{
  X t;
}

Now, which type does attribute t have? Indeed, the name of model-level enumerations must be unique in the model. The internal enumerations do not need to have this restriction. They cannot have the same name as their classes have.
This example is valid:

class X{
  enum Y{y}
  Y t; // Y is not a class here
}
class Y{
}

This example is invalid:

class X{
  enum X{x}
  X t;
}

One more case that can cause conflict. Consider the following example:

class X{
  enum Y{y}
  0..1 -- *Y;
}
class Y{
}

Now, x has an association with class Y or enumeration Y!!!. It is enumeration Y1, but we cannot have a bidirectional association between a class and enumeration. Indeed, when you generate the code, you will not see a compilation error because the definition of enumeration has priority or the definition of class Y. Class X will have many enumerations of Y and class Y will have 0..1 of class X. This is wrong because we wanted to make an association between class X and class Y. You need to detect this case and raise a proper error.
However, this example is valid:

class X{
  enum Y{y}
  0..1 -> * Y;
}
class Y{
}

In this case, because the association is directional, there is no issue. The class will make an association between class X and enumeration Y. In this case, don't make an error and just warn the user about the ambiguity and the real output (which is no association between class X and class Y, but one between class X and enumeration Y). Make sure the generated code is correct for this cases.
NOTE: enumerations and interfaces somehow have the same restriction while they are used in associations.

@jblang94

This comment has been minimized.

Contributor

jblang94 commented Apr 7, 2017

@vahdat-ab I found the following cases that need to be addressed regarding enumerations and state machines:

enum Y { Red, Blue, Green }
class X {
  showY(Y attr) { System.out.println(attr); }
  Y {
    s1 {
       goToS2 -> s2;
    }
    s2 { }
  }
}
class X {
  enum Y { Red, Blue, Green }
  Y {
     s1 {
         goToS2 -> s2;
     }
  }
}

In both cases, enumerations are generated to represent enum Y and state machine Y. I will raise an error when there is a conflict between an enumeration type and a state machine.

@jblang94

This comment has been minimized.

Contributor

jblang94 commented Apr 7, 2017

@vahdat-ab what should be the behaviour for the following cases?

Compositions

class X {
  enum Y { Red, Blue, Green }
  0..1 <@>- * Y;   // or 0..1 -<@> * Y;
}

class Y {
  name;
}

Should an error be generated in this case? I noticed in the generated code

 //------------------------
  // ENUMERATIONS
  //------------------------

  public enum Y { Red, Blue, Green }

  //------------------------
  // MEMBER VARIABLES
  //------------------------

  //X Associations
  private List<Y> ies;

Unidirectional Association

From your example above, do I issue a warning in the case that the association points the opposite way <- ?

isA

The following example when generated can be compiled by the Java compiler. However, do I need to raise a warning / error? It is ambiguous whether X attr refers to the superclass, or the enumeration

class X {}

class Y {
  enum X { Red, Blue, Green }
  X attr;
  isA X;
}
@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Apr 8, 2017

The general rule is this:
Any kind of association that needs both ends have a reference to each other must be avoided for interfaces and enums. If one side is required and it is not an enumeration and interface, it's legitimate. The reason enums and interfaces cannot be involved is that they cannot have references (as an attribute).
Your first example cannot be done with enumeration because Y needs to have a reference to X. This is an error.
The second example is valid.
Class Y is a subclass of class X and there is an attribute of type enum X named attr. You don't need to raise a warning because the semantics is clear in this case.

@jblang94

This comment has been minimized.

Contributor

jblang94 commented Apr 8, 2017

@vahdat-ab I discovered quite a few more cases, and still need more time for my solution. I won't be able to submit a PR tonight, and I have to stop my work for finals. I will resume starting April 22nd when I return from my travels.

@vahdat-ab vahdat-ab closed this Sep 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment