Skip to content

Commit 26f79f2

Browse files
ZachBrayZachBray
committed
Address Mike's feedback: move sys prop eval higher.
In this commit, I've removed the `PrecedenceChecks#Configuration` class in favour of a static method within `TargetCodeGeneratorLoader`. Unfortunately, C# behaves differently to all the other code generators. To avoid duplicating the default configuration for precedence checks in the `CSharp` class, I've made the method publicly visible, which is a bit ugly. It has also introduced a cyclic dependency between the code generators, e.g., `JavaGenerator`, and `TargetCodeGeneratorLoader`, as in order to preserve the existing public signatures I have kept the old constructors around and supplied a default `PrecedenceChecks`.
1 parent 0eb1266 commit 26f79f2

File tree

7 files changed

+44
-44
lines changed

7 files changed

+44
-44
lines changed

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/TargetCodeGeneratorLoader.java‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public CodeGenerator newInstance(final Ir ir, final String outputDir)
5454
"true".equals(System.getProperty(JAVA_GENERATE_INTERFACES)),
5555
"true".equals(System.getProperty(DECODE_UNKNOWN_ENUM_VALUES)),
5656
"true".equals(System.getProperty(TYPES_PACKAGE_OVERRIDE)),
57-
PrecedenceChecks.newInstance(new PrecedenceChecks.Context()),
57+
precedenceChecks(),
5858
new JavaOutputManager(outputDir, ir.applicableNamespace()));
5959
}
6060
},
@@ -86,7 +86,7 @@ public CodeGenerator newInstance(final Ir ir, final String outputDir)
8686
return new CppGenerator(
8787
ir,
8888
"true".equals(System.getProperty(DECODE_UNKNOWN_ENUM_VALUES)),
89-
PrecedenceChecks.newInstance(new PrecedenceChecks.Context()),
89+
precedenceChecks(),
9090
new NamespaceOutputManager(outputDir, ir.applicableNamespace()));
9191
}
9292
},
@@ -121,6 +121,30 @@ public CodeGenerator newInstance(final Ir ir, final String outputDir)
121121
}
122122
};
123123

124+
/**
125+
* Returns the configured precedence checks.
126+
*
127+
* @return the configured precedence checks.
128+
*/
129+
public static PrecedenceChecks precedenceChecks()
130+
{
131+
final boolean shouldGeneratePrecedenceChecks =
132+
Boolean.parseBoolean(System.getProperty(GENERATE_PRECEDENCE_CHECKS, "false"));
133+
134+
final String precedenceChecksFlagName =
135+
System.getProperty(PRECEDENCE_CHECKS_FLAG_NAME, "SBE_ENABLE_PRECEDENCE_CHECKS");
136+
137+
final String precedenceChecksPropName =
138+
System.getProperty(JAVA_PRECEDENCE_CHECKS_PROPERTY_NAME, "sbe.enable.precedence.checks");
139+
140+
final PrecedenceChecks.Context context = new PrecedenceChecks.Context()
141+
.shouldGeneratePrecedenceChecks(shouldGeneratePrecedenceChecks)
142+
.precedenceChecksFlagName(precedenceChecksFlagName)
143+
.precedenceChecksPropName(precedenceChecksPropName);
144+
145+
return PrecedenceChecks.newInstance(context);
146+
}
147+
124148
/**
125149
* Do a case-insensitive lookup of a target language for code generation.
126150
*

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/common/PrecedenceChecks.java‎

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
*/
1616
package uk.co.real_logic.sbe.generation.common;
1717

18-
import uk.co.real_logic.sbe.SbeTool;
1918
import uk.co.real_logic.sbe.ir.Token;
2019

2120
import java.util.ArrayList;
@@ -161,45 +160,14 @@ private FieldPrecedenceModel createModel(
161160
return null;
162161
}
163162

164-
static final class Configuration
165-
{
166-
private static final boolean GENERATE_PRECEDENCE_CHECKS_VALUE =
167-
Boolean.parseBoolean(System.getProperty(SbeTool.GENERATE_PRECEDENCE_CHECKS, "false"));
168-
169-
private static final String PRECEDENCE_CHECKS_FLAG_NAME_VALUE =
170-
System.getProperty(SbeTool.PRECEDENCE_CHECKS_FLAG_NAME, "SBE_ENABLE_PRECEDENCE_CHECKS");
171-
172-
private static final String PRECEDENCE_CHECKS_PROPERTY_NAME_VALUE =
173-
System.getProperty(SbeTool.JAVA_PRECEDENCE_CHECKS_PROPERTY_NAME, "sbe.enable.precedence.checks");
174-
175-
private Configuration()
176-
{
177-
}
178-
179-
static boolean shouldGeneratePrecedenceChecks()
180-
{
181-
return GENERATE_PRECEDENCE_CHECKS_VALUE;
182-
}
183-
184-
static String precedenceChecksFlagName()
185-
{
186-
return PRECEDENCE_CHECKS_FLAG_NAME_VALUE;
187-
}
188-
189-
static String precedenceChecksPropName()
190-
{
191-
return PRECEDENCE_CHECKS_PROPERTY_NAME_VALUE;
192-
}
193-
}
194-
195163
/**
196164
* The context describing how precedence checks should be generated.
197165
*/
198166
public static final class Context
199167
{
200-
private boolean shouldGeneratePrecedenceChecks = Configuration.shouldGeneratePrecedenceChecks();
201-
private String precedenceChecksFlagName = Configuration.precedenceChecksFlagName();
202-
private String precedenceChecksPropName = Configuration.precedenceChecksPropName();
168+
private boolean shouldGeneratePrecedenceChecks;
169+
private String precedenceChecksFlagName;
170+
private String precedenceChecksPropName;
203171

204172
/**
205173
* Returns {@code true} if precedence checks should be generated; {@code false} otherwise.

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import uk.co.real_logic.sbe.PrimitiveType;
1919
import uk.co.real_logic.sbe.generation.CodeGenerator;
2020
import uk.co.real_logic.sbe.generation.Generators;
21+
import uk.co.real_logic.sbe.generation.TargetCodeGeneratorLoader;
2122
import uk.co.real_logic.sbe.generation.common.FieldPrecedenceModel;
2223
import uk.co.real_logic.sbe.generation.common.PrecedenceChecks;
2324
import uk.co.real_logic.sbe.ir.Encoding;
@@ -71,7 +72,7 @@ public CppGenerator(final Ir ir, final boolean shouldDecodeUnknownEnumValues, fi
7172
this(
7273
ir,
7374
shouldDecodeUnknownEnumValues,
74-
PrecedenceChecks.newInstance(new PrecedenceChecks.Context()),
75+
TargetCodeGeneratorLoader.precedenceChecks(),
7576
outputManager
7677
);
7778
}

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharp.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
import uk.co.real_logic.sbe.generation.CodeGenerator;
2020
import uk.co.real_logic.sbe.generation.TargetCodeGenerator;
21-
import uk.co.real_logic.sbe.generation.common.PrecedenceChecks;
21+
import uk.co.real_logic.sbe.generation.TargetCodeGeneratorLoader;
2222
import uk.co.real_logic.sbe.ir.Ir;
2323

2424
/**
@@ -33,7 +33,7 @@ public CodeGenerator newInstance(final Ir ir, final String outputDir)
3333
{
3434
return new CSharpGenerator(
3535
ir,
36-
PrecedenceChecks.newInstance(new PrecedenceChecks.Context()),
36+
TargetCodeGeneratorLoader.precedenceChecks(),
3737
new CSharpNamespaceOutputManager(outputDir, ir.applicableNamespace()));
3838
}
3939
}

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharpGenerator.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import uk.co.real_logic.sbe.PrimitiveValue;
2121
import uk.co.real_logic.sbe.generation.CodeGenerator;
2222
import uk.co.real_logic.sbe.generation.Generators;
23+
import uk.co.real_logic.sbe.generation.TargetCodeGeneratorLoader;
2324
import uk.co.real_logic.sbe.generation.common.FieldPrecedenceModel;
2425
import uk.co.real_logic.sbe.generation.common.PrecedenceChecks;
2526
import uk.co.real_logic.sbe.ir.Encoding;
@@ -71,7 +72,7 @@ public CSharpGenerator(final Ir ir, final OutputManager outputManager)
7172
{
7273
this(
7374
ir,
74-
PrecedenceChecks.newInstance(new PrecedenceChecks.Context()),
75+
TargetCodeGeneratorLoader.precedenceChecks(),
7576
outputManager
7677
);
7778
}

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import uk.co.real_logic.sbe.PrimitiveType;
1919
import uk.co.real_logic.sbe.generation.CodeGenerator;
2020
import uk.co.real_logic.sbe.generation.Generators;
21+
import uk.co.real_logic.sbe.generation.TargetCodeGeneratorLoader;
2122
import uk.co.real_logic.sbe.generation.common.FieldPrecedenceModel;
2223
import uk.co.real_logic.sbe.generation.common.PrecedenceChecks;
2324
import uk.co.real_logic.sbe.ir.*;
@@ -130,7 +131,7 @@ public JavaGenerator(
130131
shouldGenerateInterfaces,
131132
shouldDecodeUnknownEnumValues,
132133
shouldSupportTypesPackageNames,
133-
PrecedenceChecks.newInstance(new PrecedenceChecks.Context()),
134+
TargetCodeGeneratorLoader.precedenceChecks(),
134135
outputManager
135136
);
136137
}

‎sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/java/JavaGeneratorTest.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,10 @@ void shouldGenerateBasicMessage() throws Exception
260260
@Test
261261
void shouldGenerateWithoutPrecedenceChecksByDefault() throws Exception
262262
{
263-
final PrecedenceChecks.Context context = new PrecedenceChecks.Context();
263+
final PrecedenceChecks.Context context = new PrecedenceChecks.Context()
264+
.shouldGeneratePrecedenceChecks(false)
265+
.precedenceChecksPropName("sbe.enable.precedence.checks")
266+
.precedenceChecksFlagName("SBE_ENABLE_PRECEDENCE_CHECKS");
264267
final PrecedenceChecks precedenceChecks = PrecedenceChecks.newInstance(context);
265268
generator(precedenceChecks).generate();
266269

@@ -276,7 +279,9 @@ void shouldGenerateWithoutPrecedenceChecksByDefault() throws Exception
276279
void shouldGeneratePrecedenceChecksWhenEnabled() throws Exception
277280
{
278281
final PrecedenceChecks.Context context = new PrecedenceChecks.Context()
279-
.shouldGeneratePrecedenceChecks(true);
282+
.shouldGeneratePrecedenceChecks(true)
283+
.precedenceChecksPropName("sbe.enable.precedence.checks")
284+
.precedenceChecksFlagName("SBE_ENABLE_PRECEDENCE_CHECKS");
280285
final PrecedenceChecks precedenceChecks = PrecedenceChecks.newInstance(context);
281286
generator(precedenceChecks).generate();
282287

0 commit comments

Comments
 (0)