Skip to content

Implementation of analyzer for wizards-and-warriors-2 #262

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/java/analyzer/AnalyzerRoot.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import analyzer.exercises.secrets.SecretsAnalyzer;
import analyzer.exercises.twofer.TwoferAnalyzer;
import analyzer.exercises.wizardsandwarriors.WizardsAndWarriorsAnalyzer;
import analyzer.exercises.wizardsandwarriors2.WizardsAndWarriors2Analyzer;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -33,7 +34,6 @@ private AnalyzerRoot() {
*/
public static Output analyze(Solution solution) {
var outputBuilder = new OutputBuilder();

for (Analyzer analyzer : createAnalyzers(solution.getSlug())) {
analyzer.analyze(solution, outputBuilder);
}
Expand All @@ -49,7 +49,6 @@ private static List<Analyzer> createAnalyzers(String slug) {
var analyzers = new ArrayList<Analyzer>();

analyzers.add(new GlobalAnalyzer());

switch (slug) {
case "annalyns-infiltration" -> analyzers.add(new AnnalynsInfiltrationAnalyzer());
case "hamming" -> analyzers.add(new HammingAnalyzer());
Expand All @@ -61,6 +60,7 @@ private static List<Analyzer> createAnalyzers(String slug) {
case "secrets" -> analyzers.add(new SecretsAnalyzer());
case "two-fer" -> analyzers.add(new TwoferAnalyzer());
case "wizards-and-warriors" -> analyzers.add(new WizardsAndWarriorsAnalyzer());
case "wizards-and-warriors-2" -> analyzers.add(new WizardsAndWarriors2Analyzer());
}

return List.copyOf(analyzers);
Expand Down
Copy link
Member

Choose a reason for hiding this comment

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

What does "Hardcoded" in the name refer to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It refers to the fact of using the same comment of "Reuse_code", but with the parameters "hardcoded", as we said here.
You can check this PR for website-copy.

Copy link
Member

Choose a reason for hiding this comment

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

I think I can see where you're coming from, but it might not be clear to what's "hardcoded". Could we come up with another name? The only alternative suggestions I could think was ReuseCodeFromDescribeTwoParams and ReuseCodeFromDescribeThreeParams.

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package analyzer.exercises.wizardsandwarriors2;

import analyzer.Comment;

public class ReuseCodeHardcodedThreeParameters extends Comment {

@Override
public String getKey() {
return "java.wizards-and-warriors-2.reuse_code_hardcoded_three_parameters";
}

@Override
public Type getType() {
return Type.ACTIONABLE;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package analyzer.exercises.wizardsandwarriors2;

import analyzer.Comment;

public class ReuseCodeHardcodedTwoParameters extends Comment {

@Override
public String getKey() {
return "java.wizards-and-warriors-2.reuse_code_hardcoded_two_parameters";
}

@Override
public Type getType() {
return Type.ACTIONABLE;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package analyzer.exercises.wizardsandwarriors2;

import analyzer.Analyzer;
import analyzer.OutputCollector;
import analyzer.Solution;
import analyzer.comments.ExemplarSolution;
import analyzer.comments.PreferStringConcatenation;
import com.github.javaparser.ast.body.MethodDeclaration;
import com.github.javaparser.ast.expr.MethodCallExpr;
import com.github.javaparser.ast.visitor.VoidVisitorAdapter;

import java.util.List;

/**
* The {@link WizardsAndWarriors2Analyzer} is the analyzer implementation for the {@code wizards-and-warriors-2} concept exercise.
*
* @see <a href="https://github.com/exercism/java/tree/main/exercises/concept/wizards-and-warriors-2">The wizards-and-warriors exercise on the Java track</a>
*/
public class WizardsAndWarriors2Analyzer extends VoidVisitorAdapter<OutputCollector> implements Analyzer {

private static final String EXERCISE_NAME = "Wizards and Warriors 2";
private static final String GAME_MASTER = "GameMaster";
private static final String DESCRIBE = "describe";
private static final String FORMAT = "format";

@Override
public void analyze(Solution solution, OutputCollector output) {

for (var compilationUnit : solution.getCompilationUnits()) {
compilationUnit.getClassByName(GAME_MASTER).ifPresent(c -> c.accept(this, output));
}

if (output.getComments().isEmpty()) {
output.addComment(new ExemplarSolution(EXERCISE_NAME));
}
}

@Override
public void visit(MethodDeclaration node, OutputCollector output) {

if(!node.getNameAsString().equals(DESCRIBE)) {
return;
}

if(node.getParameters().size() == 2 && !reuseMethod(node)) {
Copy link
Member

Choose a reason for hiding this comment

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

Could we determine which method is being visited more accurately? My concern with simply counting the number of parameters is that it could also match other describe methods that students might write but we don't anticipate. For example:

public class GameMaster {

        public String describe(Character character) {
            // ...
        }

        public String describe(Destination destination) {
            // ...
        }

        public String describe(TravelMethod travelMethod) {
            // ...    
        }

        public String describe(Character character, Destination destination, TravelMethod travelMethod) {
            // ... 
        }

        public String describe(Character character, Destination destination) {
            // ...
        }

        private String describe(int someNumber, String someString) {
            // Also has two parameters!
        }
}

I think one way is to have the analyzer check the parameter's types (e.g. node.getParameters().stream().map(Parameter::getTypeAsString)).

Copy link
Contributor Author

@chiarazarrella chiarazarrella May 5, 2025

Choose a reason for hiding this comment

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

I though this kind of check was already done by the tests for passing the exercise. But I can modify it by checking also the type of parameters :)

-- pushed the new changes

Copy link
Member

Choose a reason for hiding this comment

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

The tests only check that the ones that are needed are there (e.g. the public ones in above). It can't check if there are any extra unexpected ones (like the private one in my example above).


output.addComment(new ReuseCodeHardcodedTwoParameters());

}

if(node.getParameters().size() == 3 && !reuseMethod(node)) {

output.addComment(new ReuseCodeHardcodedThreeParameters());

}

if(useStringFormat(node)) {

output.addComment(new PreferStringConcatenation());

}

super.visit(node, output);
}

private static boolean reuseMethod(MethodDeclaration node) {

int paramCount = node.getParameters().size();

List<MethodCallExpr> describeCalls = node.findAll(MethodCallExpr.class).stream()
.filter(m -> m.getNameAsString().equals(DESCRIBE))
.toList();

if (paramCount == 2) {
return describeCalls.size() == 1 || describeCalls.size() == 3;
}
if (paramCount == 3) {
return describeCalls.size() == 3;
}

return false;
}

private static boolean useStringFormat(MethodDeclaration node) {
return node.findAll(MethodCallExpr.class).stream()
.anyMatch(m -> m.getNameAsString().contains(FORMAT));
}

}
16 changes: 16 additions & 0 deletions src/test/java/analyzer/AnalyzerIntegrationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -205,4 +205,20 @@ void salarycalculator(String scenario) throws IOException {

Approvals.verify(serialize(output.analysis()), Approvals.NAMES.withParameters(scenario));
}

@ParameterizedTest
@ValueSource(strings = {
"ExemplarSolution",
"NotReuseMethod",
"PartialReuseOfMethod",
"UseStringFormat",
"UseStringFormatAndNotReuseMethod",
})
void wizardsandwarriors2(String scenario) throws IOException {
var path = Path.of("wizards-and-warriors-2", scenario + ".java");
var solution = new SolutionFromFiles("wizards-and-warriors-2", SCENARIOS.resolve(path));
var output = AnalyzerRoot.analyze(solution);

Approvals.verify(serialize(output.analysis()), Approvals.NAMES.withParameters(scenario));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"comments": [
{
"comment": "java.general.exemplar",
"params": {
"exerciseName": "Wizards and Warriors 2"
},
"type": "celebratory"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"comments": [
{
"comment": "java.wizards-and-warriors-2.reuse_code_hardcoded_three_parameters",
"params": {},
"type": "actionable"
},
{
"comment": "java.wizards-and-warriors-2.reuse_code_hardcoded_two_parameters",
"params": {},
"type": "actionable"
},
{
"comment": "java.general.feedback_request",
"params": {},
"type": "informative"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"comments": [
{
"comment": "java.wizards-and-warriors-2.reuse_code_hardcoded_two_parameters",
"params": {},
"type": "actionable"
},
{
"comment": "java.general.feedback_request",
"params": {},
"type": "informative"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"comments": [
{
"comment": "java.general.prefer_string_concatenation",
"params": {},
"type": "informative"
},
{
"comment": "java.general.feedback_request",
"params": {},
"type": "informative"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"comments": [
{
"comment": "java.wizards-and-warriors-2.reuse_code_hardcoded_three_parameters",
"params": {},
"type": "actionable"
},
{
"comment": "java.wizards-and-warriors-2.reuse_code_hardcoded_two_parameters",
"params": {},
"type": "actionable"
},
{
"comment": "java.general.prefer_string_concatenation",
"params": {},
"type": "informative"
},
{
"comment": "java.general.feedback_request",
"params": {},
"type": "informative"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
public class GameMaster {

public String describe(Character character) {
return "You're a level " + character.getLevel() + " " + character.getCharacterClass() + " with " + character.getHitPoints() + " hit points.";
}

public String describe(Destination destination) {
return "You've arrived at " + destination.getName() + ", which has " + destination.getInhabitants() + " inhabitants.";
}

public String describe(TravelMethod travelMethod) {
if (travelMethod == TravelMethod.WALKING) {
return "You're traveling to your destination by walking.";
}
return "You're traveling to your destination on horseback.";
}

public String describe(Character character, Destination destination, TravelMethod travelMethod) {
return describe(character) + " " + describe(travelMethod) + " " + describe(destination);
}

public String describe(Character character, Destination destination) {
return describe(character, destination, TravelMethod.WALKING);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
public class GameMaster {

public String describe(Character character) {
return "You're a level " + character.getLevel() + " " + character.getCharacterClass() + " with " + character.getHitPoints() + " hit points.";
}

public String describe(Destination destination) {
return "You've arrived at " + destination.getName() + ", which has " + destination.getInhabitants() + " inhabitants.";
}

public String describe(TravelMethod travelMethod) {
if (travelMethod == TravelMethod.WALKING) {
return "You're traveling to your destination by walking.";
}
return "You're traveling to your destination on horseback.";
}

public String describe(Character character, Destination destination, TravelMethod travelMethod) {
return "You're a level " + character.getLevel() + " " + character.getCharacterClass() + " with " + character.getHitPoints() + " hit points. " +
(travelMethod == TravelMethod.WALKING ? "You're traveling to your destination by walking. " : "You're traveling to your destination on horseback. ") +
"You've arrived at " + destination.getName() + ", which has " + destination.getInhabitants() + " inhabitants.";
}

public String describe(Character character, Destination destination) {
return "You're a level " + character.getLevel() + " " + character.getCharacterClass() + " with " + character.getHitPoints() + " hit points. " +
"You're traveling to your destination by walking. " +
"You've arrived at " + destination.getName() + ", which has " + destination.getInhabitants() + " inhabitants.";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
public class GameMaster {

public String describe(Character character) {
return "You're a level " + character.getLevel() + " " + character.getCharacterClass() + " with " + character.getHitPoints() + " hit points.";
}

public String describe(Destination destination) {
return "You've arrived at " + destination.getName() + ", which has " + destination.getInhabitants() + " inhabitants.";
}

public String describe(TravelMethod travelMethod) {
if (travelMethod == TravelMethod.WALKING) {
return "You're traveling to your destination by walking.";
}
return "You're traveling to your destination on horseback.";
}

public String describe(Character character, Destination destination, TravelMethod travelMethod) {
return describe(character) + " " + describe(travelMethod) + " " + describe(destination);
}

public String describe(Character character, Destination destination) {
return describe(character) + " " + describe(destination) + " You're traveling to your destination by walking.";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
public class GameMaster {

public String describe(Character character) {
return "You're a level %d %s with %d hit points.".formatted(character.getLevel(),
character.getCharacterClass(), character.getHitPoints());
}

public String describe(Destination destination) {
return "You've arrived at %s, which has %d inhabitants.".formatted(destination.getName(),
destination.getInhabitants());
}

public String describe(TravelMethod travelMethod) {
if (travelMethod == TravelMethod.WALKING) {
return "You're traveling to your destination by walking.";
}
return "You're traveling to your destination on horseback.";

}

public String describe(Character character, Destination destination, TravelMethod travelMethod) {
return "%s %s %s".formatted(describe(character), describe(travelMethod), describe(destination));
}

public String describe(Character character, Destination destination) {
return describe(character, destination, TravelMethod.WALKING);
}
}
Loading