#936 Symbols/Facet Redesign

brian Tue 26 Jan 2010

This proposal is aligned with #934 and #935 as part of a push to get architectural breaking changes finalized.

Today the design for facets is based on the Symbols - ticket #564. The goal was to model the name of a facet not the name/value pair, and provide some unification with other "named things" like configuration and localization.

The existing symbol design was put in place last summer, and after about six months experience with it, I would say it is a failed experiment. I don't like the extra complexity it adds to core pod::Type#slot namespace.

Andy and I have kicked around a few ideas, and I think the path of least resistance is just copy Java annotation and C# attribute model. While I'm not huge fan of that model, I think if we wish to support namespace/type checked facets then the obvious solution is to use an actual Type for the facet. That leaves two choices: use a Type for the name as a marker, or actually use a Type for the name/value pair like Java/C# uses. The latter seems like the better choice mostly because I think it will ease FFI impedance mismatches - see ticket #668.

I also think this might work into type/slot code generation DSLs better.

This proposal is to remove Symbols in their entirety. Existing declarations in pod.fan will go away as will the fcode and reflection support for symbols. Symbols are used in two ways today:

  • name keys for facets
  • configuration properties

New Facets Design

Facets are replaced with a normal type:

abstract const class Facet {}

const class Transient : Facet {}

const class PodDepends : Facet
{
  new (Depend[] v) { this.val = v }
  const Depend[] val
}

Definition of facets looks like constructor calls:

@PodDepends([Depend("sys 1.0"), Depend("web 1.0")])
@PodSrcDirs([`fan/`, `test/`])
@PodJsDirs([`js/`])
@Js
@Docsrc
pod dom {}

Reflection APIs change:

Facet[] facets()
Facet? facet(Type t, Bool checked := true)

Not sure I love it, but simplifies the architecture to not have namespaced elements other than types under pods. Some things to consider:

  • maybe we should use lower case facet type names?
  • force use of a single val for every facet more like existing name=val design

Config/Localization Also as part of this proposal is drop back down to a simpler configuration mechanism based strictly on Str/Str value pairs. This includes removing the "fansym" format and its associated IO APIs and stick with "props" files for both configuration and localization. These APIs will probably be reworked slightly to provide appropiate hooks in Env.

brian Tue 26 Jan 2010

Promoted to ticket #936 and assigned to brian

qualidafial Tue 26 Jan 2010

+1. I've been uneasy about the symbols design for a while, although it seemed like a good idea at the time. :)

maybe we should use lower case facet type names?

I think there's an advantage in having facet names look distinct from type names, so +1.

force use of a single val for every facet more like existing name=val design

That will probably be the most common approach, but I'm not sure it's necessary to enforce it. If I'm writing a more complex facet (2 or more fields) I don't necessary want to write a new class just to hold the values for the facet--I may as well just put those fields in the facet.

@Foo(bar:"blah", baz:42)
class Buz { ... }

I chose a map-like syntax here because it looks nice but it could just as well be given using a standard, serializable block:

@Foo { bar="blah"; baz=42; }
class Buz { ... }

brian Wed 27 Jan 2010

Anymore comments on this? I'm finding it very hard to believe :-)

Assuming we switch Facet to be a normal type, should we use lower case or capitalized type names:

@transient  vs @Transient
@js         vs @Js
@podDepends vs @PodDepends

tactics Wed 27 Jan 2010

I think we should stick to caps names. Its a class, afterall, and all other classes are capped.

Also, I don't quite understand what you mean by "forcing the use of a single val". Could you clarify?

Other than that, this proposal seems fine by me.

brian Wed 27 Jan 2010

Also, I don't quite understand what you mean by "forcing the use of a single val". Could you clarify?

Current facets are a name/value pair such as:

@Foo="bar"
class Baz {}

If we limit all Facets to a single value then we can stick with that model. Or we can allow a given Facet class to have as many fields as it likes and use a constructor syntax:

@Foo("bar")
class Baz {}

I personally find the = syntax a lot easier to read (if you look at some of your pod.fan files). But it doesn't really fit the Java/C# model.

andy Wed 27 Jan 2010

forcing the use of a single val

I don't like that, lets keep normal ctor and ctor-with-block usage.

tactics Wed 27 Jan 2010

Ah. I see what you're getting at now. I'm just going to think out loud here.

I agree that the = sign looks nicer. It looks like you're tagging a pod, class, slot, whatever with an annotation.

I still think if a facet is just a type, it should be spelled in uppercase like all other types.

They are specialized enough that they could probably use a special declaration syntax, like pods and enums. Something like:

facet Opt : Str {}

Which would automatically expand to the class declaration

class Opt : Facet
{
  Str val
  new make(Str val) { this.val = val }
}

Is there any way we can keep config/localized data in the .fob format?

jodastephen Wed 27 Jan 2010

I do think that the class based approach will be the most powerful long term. That said, I like the idea of a dedicated class type:

facet Opt {
  Str val
}

which generates the appropriate code.

I'm not a big fan of the = syntax, when the constructor syntax is close and also simple.

I'm happier with regular UpperCamel facet names.

I haven't understood the impact on localization yet.

KevinKelley Wed 27 Jan 2010

+1, seems like letting facets be arbitrary Facet subclasses should take care of static meta-data.

If facets are types, constructor syntax for initializers makes sense.

Simple tag facets like @transient seem nice in lower-case, but that makes it look more like a keyword than an object type. Then again, I don't see a need to enforce case on class names.

brian Wed 27 Jan 2010

Is there any way we can keep config/localized data in the .fob format?

There was a lot of complexity associated with making localization/config values typed as something other than String. It is a lot more simple, safer, and efficient to just have the infrastructure manage string/string pairs.

But remember it is super easy to to decode a string:

fansh> "87".in.readObj
87
fansh> "5min".in.readObj
5min
fansh> "true".in.readObj
true

msl Wed 27 Jan 2010

WRT the case issue - is a facet an instance of a class (lower case), or is it a type (upper case), or is it a constructor (upper case)?

My gut feeling is that it's a constructor with properties passed to it, in which case @UpperCase seems the way to go for consistency...

andy Wed 27 Jan 2010

I think we need to keep normal type-case here, its an unnecessary anomaly otherwise. So I'm on board with this proposal.

katox Wed 27 Jan 2010

Is there a need to have facets typed? What is the main reason? I remember pulling out hair when using typed annotations with enum types...

msl Thu 28 Jan 2010

Is there a need to have facets typed? What is the main reason?

The biggest benefits I see when using strongly typed facets in Java, such as JPA's

@Temporal(TemporalType.DATE)

over weakly typed ones, such as

@SuppressWarnings({"serializable", "unchecked"})

are:

  1. Compile time checking of the validity of annotation values (ie typos)
  2. Tool (IDE) integration to provide better assistance in annotation application

I guess the key thing it gives me (as a lazy programmer) is static checks that I've got things relatively correct at a syntactic level. It follows in the footsteps of static typing eliminating a whole range of errors that you need to check for and test in dynamic languages.

ivan Thu 28 Jan 2010

If you introduce a base class Facet, probably there should be base classes for Type/Slot facets? So there can be compile-time check of valid facet usage?

msl Thu 28 Jan 2010

Or atleast some way to specify that...

enum FacetScope { pod, facet, type, slot }
abstract class Facet {
  virtual FacetScope[] scope := [pod, type, slot]
}
class MyFacet : Facet {
  override FieldScope[] scope := [pod]
}

or perhaps a more recursive mechanism:

@Scope([pod, facet, type, slot])
abstract class Facet {}
@Scope(pod)
class MyFacet : Facet {...}

or taking previous comments into account (my preference):

@Scope(pod)
facet MyFacet {...}

Yuri Strot Thu 28 Jan 2010

I like the original proposal and uppercase for all types.

However I think it's more flexible to have special keyword for facet like class/mixin. For instance it allow to have general base class:

//shouldn't be a facet
const class BaseFacet
{
  new make(|This|? f := null) { f?.call(this) }
  Str name := ""
  Str description := ""
  Int id := 0
}

facet Extension: BaseFacet
{
  new make(|This|? f) : super() { f?.call(this) }
  Float priority := 1f
}

facet Condition: BaseFacet
{
  new make(|This|? f) : super() { f?.call(this) }
  ...
}

Also for such facets it makes more sense to use lower case (no need for type consistence):

@simple
final class Depend
{
...
}

facet podDepends
{
  new make(Depend[] v) { this.val = v }
  Depend[] val
}

From the other hand, facets doesn't look for me as any other classes: is it make sense to have internal or mutable facets? I believe facets need another set of modifiers (scope, etc.)

Anyway original proposal looks fine.

andy Thu 28 Jan 2010

Regarding the facet syntax:

  1. facet Foo vs const class Foo : Facet insisted a big enough difference to warrant taking a keyword over.
  2. const class Foo : Facet makes it clearer that facets must be const classes.

Adding syntax should be reserved for things that really cut down on boiler plate code, and this just doesn't seem to be a candidate for that IMO.

DanielFath Thu 28 Jan 2010

From the docs

Enums are a special type of class that define a discrete range of possible values:

* Enums are normal classes with all associated characteristics
* Enums are implied const
* Enums are implied final

Now if you don't write const final class MyEnum : Enum, why would you write all that for Facet? Ok so it has 5 letters more but I don't see that is too big difference to distinguish these two cases.

tactics Thu 28 Jan 2010

Adding syntax should be reserved for things that really cut down on boiler plate code, and this just doesn't seem to be a candidate for that IMO.

Facets occupy a different semantic space than regular classes. You're never going to instantiate a facet in a method. You're never going to subclass them. They never appear as slots and you never see them passed as parameters. Something about their declaration should yell out, "this is no ordinary class!"

brian Thu 28 Jan 2010

The difference between Facets and Enum is in actual syntax grammar which requires a keyword at the syntax level. I don't believe the syntax grammar of a facet will be any different than a normal type.

Although I actually don't like taking enum as a keyword, and eventually I want to add first class bitmask support. So I'm been thinking of changing enum to be a special modifier stuck before class:

enum class Month { ... }
bitmask class Modifiers { ... }
facet class Transient { ... }

jodastephen Thu 28 Jan 2010

Both mixins and enums could also be expressed using just classes and the inheritance syntax. The point is about expressing this is a key component of the language. A keyword is good for that. I would also expect it to enforce the const nature and generate the appropriate constructors.

andy Thu 28 Jan 2010

I don't see that is too big difference to distinguish these two cases

There is a very big difference - the only thing special about facets are they extend Facet. Enums handle all the boilerplate of creating the appropriate ordinal and values lists with all the appropriate naming.

jodastephen Thu 28 Jan 2010

I would strongly support enum class and facet class combinations. Especially as they would allow enum and facet to not be keywords (they would be context sensitive ones).

brian Wed 3 Feb 2010

I'm still working on the new facet design; still not sure how it all goes together. But a couple of things Andy and I have talked about...

First, I am going to change the syntax for "special" classes to use a positional keyword:

enum class Month {...}
facet class Transient {...}

Eventually we could add something like struct class to do the auto equals/hash/ctor thing. This change will release the "enum" keyword (and avoid taking "facet" as a keyword).

Looking through all the facets, we decided that if facets were going to move towards normal OO classes that would we coalesce many of the existing facets into a single class. For example we discussed that most all of the pod facets will be collapsed into PodBuild for all the stuff you define for build tools (depends, dirs, etc) and then PodMeta for all the standard meta-data stuff the build generates from PodBuild (stuff like version, depends, build time, host, etc). For example what "fwt/pod.fan" would look like:

@PodBuild 
{ 
  depends  = ["sys 1.0", "gfx 1.0"]
  srcDirs  = [`fan/`, `test/`]
  javaDirs = [`java/`]
  jsDirs   = [`js/`]
  resDirs  = [`locale/`, `res/img/`, `res/javafx/`]
}
@Js
@DocSrc
pod fwt {}

I am still struggling with the facet that facets can't just be any old OO class - the compiler has to understand their serialized value without evaluating the constructor during compile time. I'm sort of thinking that you can do whatever you want in your facet class except you can't a constructor. The compiler will handle that using you declared fields:

Singleton:

facet class Transient {}   
@Transient   // sugar for Transient.defVal

Single Val:

facet class UriScheme { const Str val }
@UriScheme("fan")  

Multiple Vals:

facet class Foo { const Int a; const Int b }
@Foo { a=4; b=6; }

Enum:

facet enum class Serializable { simple, complex, collection }
@Serializable.simple

How does that look to everybody?

Also not sure about how to work new model into type database yet. One thing I am going to do is have compiler generate a simpler indexing file (probably a props file), so it is easier to work with. As part of this change I will be delegating type database lookups to the Env.

EDIT: add class keyword to examples

andy Wed 3 Feb 2010

That construction model makes sense to me.

jodastephen Wed 3 Feb 2010

Perhaps this might be a syntax that is easier to parse for these classes:

class Person { ... }
class(enum) Sex { ... }
class(facet) UriScheme { ... }
class(enum,facet) Serializable { ... }

Its not typical of the rest of the language though.

I also wonder if facets need to specify that their fields are const, simply because they have to be const.

I also think the difference between the round-bracket and curly-bracket construction forms for a facet look quite subtle. Is the full construction syntax block required here, including implicit calls to add() and other methods? Or would a simpler "named parameters" style be sufficient? The latter might be safer and easier to parse/manage.

brian Wed 3 Feb 2010

I also wonder if facets need to specify that their fields are const, simply because they have to be const.

Are you suggesting that the fields are implied to be const? That might be nice, but we don't do that today for const classes, enums, or static fields (which are all required to be const too). So I'd stick with making it explicit.

I also think the difference between the round-bracket and curly-bracket construction forms for a facet look quite subtle.

Many facets only have the single val field, so I thought it might be useful to add support for () syntax. But I think certainly we can just force everything to use {} syntax:

@UriScheme { scheme="fan" }

That might be a nice simple way to start.

jodastephen Wed 3 Feb 2010

Many facets only have the single val field,

I was really asking whether we could use this syntax:

@Foo(a=4, b=6)

or whether the advanced method-calling ability of the block is required? I ask, because the above would be a lot simpler to manage during compile time. (note that I suspect that we do need the advanced block syntax, but its worth asking).

By the way, the way in which Java treats value as special in annotations is frequently considered to be a problem, because it makes migration (adding a second field) difficult (as it breaks all existing users of the annotation.

brian Wed 3 Feb 2010

or whether the advanced method-calling ability of the block is required?

It is all going to be specially handled by the compiler anyways. So I think the {} syntax would be more consistent for named constructors.

By the way, the way in which Java treats value as special in annotations is frequently considered to be a problem,

Good point - I was thinking about that too. Maybe we just say that it is singleton (Transient), enum/bitmask, or fields using {} syntax. No special support for single val facets.

I like that design.

qualidafial Thu 4 Feb 2010

+1, I like this direction. Not sure about facet enum class { ... } though--to stay consistent with how other facets are declared on types/slots I'd prefer having a facet and a separate enum for the value:

enum SerializationStrategy { simple, complex, collection }
facet Serializable : Facet
{
  const SerializationStrategy strategy;

  new make(SerializationStrategy strategy)
  {
    this.strategy = strategy;
  }
}

thus:

@Serializable(simple)
class Duration
{
  Str toStr() { ... }
  static Duration fromStr(Str str) { ... }
}

ivan Thu 4 Feb 2010

My thought on facet syntax is that it should look like making any other objects, so ideal way is to support both Fan serialization syntax and constructor calls. For example above:

@Serializable(SerializationStrategy.simple)
class Duration 
{
  ...
}

Or

@Serializable { strategy = SerializationStrategy.simple }
class Duration...

Another thought - probably facets should have map-like syntax?

@Foo[a:4, b:5]
class Bar
{
}

msl Thu 4 Feb 2010

Not sure about facet enum class { ... } though--to stay consistent with how other facets are declared on types/slots I'd prefer having a facet and a separate enum for the value

and

My thought on facet syntax is that it should look like making any other objects, so ideal way is to support both Fan serialization syntax and constructor calls

I vote for both of these suggestions in combination. I think the more it looks like a standard constructor (just in the wrong place, and with an @ in front) the better.

No special support for single val facets.

I think that's a good thing. The less I need to remember the better!

brian Thu 4 Feb 2010

I like this direction. Not sure about facet enum class { ... } though--to stay consistent with how other facets are declared on types/slots I'd prefer having a facet and a separate enum for the value

I think its the wrong direction to pollute the API namespace with both a Facet and an Enum/Bitstring everytime you want an enumerated facet. But right now I only have one use case, so I'm thinking I might defer that.

so ideal way is to support both Fan serialization syntax and constructor calls

Creating facet instances is an extremely special case. The compiler will not allow you to create your own constructor. So there are only two choices - if you declare no fields, then you get a singleton, if you declare some fields then the compiler will generate a constructor with an it-block, and that is the required syntax for construction. Not that we don't make it more flexible in the future, but I think that covers all the general cases really well, and still gives me the restrictions I need to figure out the serialized value without an eval.

jodastephen Thu 4 Feb 2010

I find the facet enum combination style really neat. It looks clear and is easily understood from its parts. (Of course both the underlying Facet and Enum class have to be mixins for this to work)

I assume that a facet writer can declare a default value for a field?

I think its OK to not define constructor syntax for facets right now, because it can be added later.

brian Thu 4 Feb 2010

I assume that a facet writer can declare a default value for a field?

Enum will remain a class, but Facet is a marker mixin (no methods).

I assume that a facet writer can declare a default value for a field?

Yeap, it will be encoded using normal Fantom serialization, so adding new fields will have binary backward compatibility.

brian Sat 6 Feb 2010

Ticket resolved in 1.0.51

This work is complete. Build coming shortly.

Login or Signup to reply.