Implementation of analyzer for wizards-and-warriors-2

[chiarazarrella]

Implementation and testing of the analyzer for the concept exercise wizards-and-warriors-2.

Closes #116

ToDo:

• Add website-copy comments
• check add comment for analyzer wizards-and-warriors-2 website-copy#2371

[kahgoh]

I think we don't need to tell students to use method overloading - the exercise has been designed so that the GameMaster.describe must be overloaded. So, all passing solutions should already be using method overload to pass.

[SleeplessByte]

Are you certain?

Is it not possible to write the solution like:

public String describe(Object...arguments) {
  // ...
}

...because if it is, I guarantee you, eventually someone will as overloading does not exist in all programming languages!

[kahgoh]

Looking at your tests for the "not use", "partial" and "use string format not use" method overloading, I'm a little confused into how method overloading isn't being used in their samples because the describe method is overloaded (defined multiple times, with different parameter types). Did you mean reuse method?

[chiarazarrella]

Oh! I see; what I meant was to exploit method overloading and yes, it is done indeed by reusing the methods.

However, I have noticed that it already exists something similar, the reuse_code comment. Should this be used instead? Or is it enough to only modify the name of the comment from use_method_overloading to reuse_method?

[kahgoh]

I think, if we can, we should use the same reuse_code comment. The comment needs to be specific about which describe method it should use though because all the methods are named describe. I'd suggest including the parameter's type in the comment. For example:

The describe(Character, Destination) method should reuse the logic implemented in describe(Character, Destination, TravelMethod). Reusing existing methods can help make code easier to maintain.

The methods can be passed as parameters to the comment (see AnalyzerIntegrationTest.loglevels.NoReuseOfBothMethods.approved.txt for an example).

[chiarazarrella]

Okay. I have a little doubt about the following, from the design.md:
If the student does not reuse the method describe(Character), describe(Destination), describe(TravelMethod) or describe(Character, Destination, TravelMethod) to solve the describe(Character, Destination) method, instruct them to do so.

Should we provide two comments? or could we modify the comment to inject also a sentence? Because I have seen that the reuse_code comment takes as input a string, we could manage to create a comment like this:

The describe(Character, Destination) method should reuse the logic implemented in describe(Character, Destination, TravelMethod) or in the individual methods describe(Character), describe(Destination), describe(TravelMethod). Reusing existing methods can help make code easier to maintain.

This means that the parameter acting as %<methodToCall>s will be a whole sentence:
describe(Character, Destination, TravelMethod) or in the single methods describe(Character), describe(Destination), describe(TravelMethod).

[kahgoh]

I think using two comments is fine and is the easier option. If we change the parameter to allow sentences as a parameter, we should check that the comments made in still make sense (because the parameter is enclosed in back ticks in the markdown).

The other possibility is to use a generic comment that can cover all the cases. For example:

The describe(Character, Destination) method should reuse the logic implemented in describe(Character, Destination, TravelMethod), describe(Character), describe(Destination) or describe(TravelMethod). Reusing existing methods can help make code easier to maintain.

[chiarazarrella]

We should handle the backticks even with two comments, though. Especially, if we need to keep the highlight of the methods only.

For example, with the following overloaded method describe(Character, Destination, TravelMethod), the comment would be:

The describe(Character, Destination, TravelMethod) method should reuse the logic implemented in describe(Character), describe(Destination) and describe(TravelMethod). Reusing existing methods can help make code easier to maintain.

and the injected string would be with the handle of the backticks:

describe(Character)`, `describe(Destination)`, and `describe(TravelMethod)

[kahgoh]

Oh, I see what you mean! I think that could work, but I'd lean towards making a copy of the comment specifically for this case. For example, the entire comment would be (no parameters in the comment):

The `describe(Character, Destination, TravelMethod)` method should reuse the logic implemented in `describe(Character)`, `describe(Destination)` and `describe(TravelMethod)`. Reusing existing methods can help make code easier to maintain.

If we close and re-open back ticks in the parameters and someone make changes to the comment's format in the future, they will need to be aware of the usage here. For example, if the change was to make methods italic, like describe(Character), the comment would look like this:

The _`%<callingMethod>s`_ method should reuse the logic implemented in _`%<methodToCall>s`_.
Reusing existing methods can help make code easier to maintain.

We'd need to remember to update the parameter accordingly, otherwise the commas and the word and would also be in italics.

describe(Character)`_, _`describe(Destination)`_, and _`describe(TravelMethod)

If we make a copy of the comments with the hard coded methods, we still need to remember to update the copy. But, if we forget to update it, at least the formatting wouldn't look weird for the comment.

[chiarazarrella]

Okay, I agree on that!
Then, I will create these two specific comments and update you asap when everything is done. :)

[SleeplessByte]

Drive-by review from a guardian: we use Git attribution and config.json attribution. Highly recommend not adding @author tags as it will lead to bikeshedding when to add/remove/ping people in the future.

[SleeplessByte]

Are you certain?

Is it not possible to write the solution like:

public String describe(Object...arguments) {
  // ...
}

...because if it is, I guarantee you, eventually someone will as overloading does not exist in all programming languages!