Skip to content
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

Add a static TYPE variable to java models #201

Merged
merged 1 commit into from
May 15, 2019

Conversation

RicoYao
Copy link
Contributor

@RicoYao RicoYao commented May 11, 2019

Example:

public class Board {
  public static final String TYPE = "board";
}

This can be useful when one needs a string-based representation of the model type.

Copy link
Contributor

@shashachu shashachu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. i originally thought it might be nice to camel or snake case the string names, but probably not necessary.

@@ -30,6 +30,10 @@ public struct JavaModelRenderer: JavaFileRenderer {
}
}

func renderStaticTypeString() -> JavaIR.Property {
return JavaIR.Property(annotations: [], modifiers: [.public, .static, .final], type: "String", name: "TYPE", initialValue: "\"" + className.lowercased() + "\"")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value of this variable should mirror what we use polymorphicTypeIdentifier for in Objective-C.

You should use the typeIdentifier getter on SchemaObjectRoot here since we allow the type strings to vary if algebraicTypeIdentifier is specified (which is intended to override the value of type in schemas if needed)

If you look at the ObjC implementation of polymorphicTypeIdentifier you will see it uses this getter method in SchemaObjectRoot

public struct SchemaObjectRoot: Equatable {
   ....
    var typeIdentifier: String {
        return algebraicTypeIdentifier ?? name
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the catch! Done.

Also ran make integration_test to update the samples.

@rahul-malik rahul-malik merged commit f368cd1 into pinterest:master May 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants