Re: [gef-dev] Zest FX UI contribution

[Fabian Steeg <fsteeg@xxxxxxxxx>]

> Am 04.07.2014 13:55, schrieb Ujhelyi Zoltán:
>> 
>> 
>> Finally, I have a code-level suggestion as well: I really don't like the static Attr class of the Graph class. It contains two enums, where the values are (1) not the complete set of supported keys/values, and (2) not all key-value pairs are supported... This is especially problematic for values. I either suggest using constants instead of enums, or providing more enums for values, and keeping only corresponding values in a single enum (e.g. LINE_DASH, LINE_DOT, LINE_SOLID, LINE_DASHDOT, LINE_DASHDOTDOT as LINE_STYLES , that can apply to both edges and node borders or DIRECTED/UNDIRECTED as both a graph and edge parameter).

On 04.07.2014, at 15:24, Matthias Wienand <Matthias.Wienand@xxxxxxxxx> wrote:

> In regards to the static Attr class of the Graph class, I am
> discontented, too. One more thing that bugs me here, is that you have to
> convert the keys to strings in order to use them to access the
> attributes map. I would opt for removing the Attr class altogether and
> provide the keys which are examined by Zest.FX as constants somewhere in
> the Zest.FX package.

I don't have a strong opinion about the Attr class and its enums. I had set it up the way it is to keep the option of using arbitrary keys and values (hence strings), but also provide some guidance in the API on what options are available (hence enums). It sounds reasonable to remove these from the Graph class as Matthias suggests, so the Graph itself only knows about strings, and the specific literals for keys and values are defined outside of it (e.g. DOT-specific attributes in the DOT component, FX-specific attributes in the FX component). There, they could be implemented as enums with corresponding values that can be used with specific keys as Zoltán suggests.