-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fixes #330 #374
base: 2024-06
Are you sure you want to change the base?
Fixes #330 #374
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Code Health Quality Gates: FAILED
Change in average Code Health of affected files: +0.36 (7.01 -> 7.38)
- Declining Code Health: 3 findings(s) 🚩
- Improving Code Health: 4 findings(s) ✅
final Double f = constantFrom == null ? value(scope, from) : constantFrom; | ||
final Double t = constantTo == null ? value(scope, to) : constantTo; | ||
Double s = constantStep == null ? value(scope, step) : constantStep; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ New issue: Complex Method
LoopStatement.FloatBounded.runIn has a cyclomatic complexity of 16, threshold = 9
public Object runIn(final IScope scope) throws GamaRuntimeException { | ||
final Object[] result = new Object[1]; | ||
final Integer f = constantFrom == null ? value(scope, from) : constantFrom; | ||
final Integer t = constantTo == null ? value(scope, to) : constantTo; | ||
Integer s = constantStep == null ? value(scope, step) : constantStep; | ||
boolean shouldBreak = false; | ||
if (f.equals(t)) { | ||
loopBody(scope, f, result); | ||
} else if (f > t) { | ||
if (s > 0) { | ||
if (stepDefined) { | ||
loopBody(scope, f, result); | ||
return result[0]; | ||
} | ||
s = -s; | ||
} | ||
for (int i = f, n = t; i >= n && !shouldBreak; i += s) { | ||
FlowStatus status = loopBody(scope, i, result); | ||
switch (status) { | ||
case CONTINUE: | ||
continue; | ||
case BREAK, RETURN, DIE, DISPOSE: | ||
shouldBreak = true; | ||
break; | ||
default: | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ New issue: Complex Method
LoopStatement.IntBounded.runIn has a cyclomatic complexity of 16, threshold = 9
IExpression from = getFacet(IKeyword.FROM); | ||
IExpression to = getFacet(IKeyword.TO); | ||
IExpression step = getFacet(IKeyword.STEP); | ||
final boolean isBounded = from != null && to != null; | ||
boolean isInt = isBounded && from.getGamlType() == Types.INT && to.getGamlType() == Types.INT | ||
&& (step == null || step.getGamlType() == Types.INT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ New issue: Complex Method
LoopStatement has a cyclomatic complexity of 10, threshold = 9
final Number f = constantFrom == null ? getFromExp(scope, from) : constantFrom; | ||
final Number t = constantTo == null ? getFromExp(scope, to) : constantTo; | ||
Number s = constantStep == null ? getFromExp(scope, step) : constantStep; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ No longer an issue: Bumpy Road Ahead
LoopStatement.Bounded.runIn is no longer above the threshold for logical blocks with deeply nested code
public Object runIn(final IScope scope) throws GamaRuntimeException { | ||
final Object[] result = new Object[1]; | ||
final Integer f = constantFrom == null ? value(scope, from) : constantFrom; | ||
final Integer t = constantTo == null ? value(scope, to) : constantTo; | ||
Integer s = constantStep == null ? value(scope, step) : constantStep; | ||
boolean shouldBreak = false; | ||
if (f.equals(t)) { | ||
loopBody(scope, f, result); | ||
} else if (f > t) { | ||
if (s > 0) { | ||
if (stepDefined) { | ||
loopBody(scope, f, result); | ||
return result[0]; | ||
} | ||
s = -s; | ||
} | ||
for (int i = f, n = t; i >= n && !shouldBreak; i += s) { | ||
FlowStatus status = loopBody(scope, i, result); | ||
switch (status) { | ||
case CONTINUE: | ||
continue; | ||
case BREAK, RETURN, DIE, DISPOSE: | ||
shouldBreak = true; | ||
break; | ||
default: | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ℹ New issue: Bumpy Road Ahead
LoopStatement.IntBounded.runIn has 3 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function
final Double f = constantFrom == null ? value(scope, from) : constantFrom; | ||
final Double t = constantTo == null ? value(scope, to) : constantTo; | ||
Double s = constantStep == null ? value(scope, step) : constantStep; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ℹ New issue: Bumpy Road Ahead
LoopStatement.FloatBounded.runIn has 3 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function
@@ -1,7 +1,6 @@ | |||
/******************************************************************************************************* | |||
* | |||
* LoopStatement.java, in gama.core, is part of the source code of the GAMA modeling and simulation platform | |||
* (v.2.0.0). | |||
* LoopStatement.java, in gama.core, is part of the source code of the GAMA modeling and simulation platform (v.2.0.0). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Getting better: Overall Code Complexity
The mean cyclomatic complexity decreases from 5.44 to 5.24, threshold = 4
final Number f = constantFrom == null ? getFromExp(scope, from) : constantFrom; | ||
final Number t = constantTo == null ? getFromExp(scope, to) : constantTo; | ||
Number s = constantStep == null ? getFromExp(scope, step) : constantStep; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ No longer an issue: Deep, Nested Complexity
LoopStatement.Bounded.runIn is no longer above the threshold for nested complexity depth
Separates the int and float bounded definitions. Makes a loop where "from" > "to" at least execute once with the value of "from". Should fix #330.