FizzBuzz evil coder chess

[felipecao]

Hi, @tobias-reuter!

I've setup our project and have written the first failing test. I've tried to describe in as much detail as possible the idea I had for an approach to the problem at the test class. Let me know if you have a better idea, or if you have any trouble setting up the project in your IDE.

Now it's your turn! I'm waiting for the next failing test! :)

[tobias-reuter]

@felipecao pong + ping for upper boundary two

[felipecao]

@tobias-reuter back at you! upper boundary 3 should display a Fizz!

[tobias-reuter]

@felipecao your turn ;-)

Maybe you can turn on the following notifications: https://help.github.com/articles/receiving-email-notifications-for-pushes-to-a-repository/

[felipecao]

@tobias-reuter you're on! :)

btw, i don't mind turning on the notifications, as long as you don't mind getting unrelated notifications every once in a while -- as you probably noticed, this repo is supposed to contain several folders with different katas, so, every time is push performed to any folder you'll be notified. By now this repo has only two folders, and if it gets too annoying, just ask me to disable it :)

BTW, I'm going to need your e-mail address to enable the push notifications.

(p.s.: i don't know about you, but I'm really having fun with this "chess" format for TDD'ing :) )

[tobias-reuter]

@felipecao go for it!

You will find my email address in every commit ;-)

I really like it so far. :-)

[felipecao]

@tobias-reuter your turn!

btw, i've configured the mail notification, let's see if it'll work from now on :)

[tobias-reuter]

@felipecao your turn!

notifications are working fine!

Wasn't sure about the refactoring I did. Do you think the step was too big?

[felipecao]

@tobias-reuter the refactoring looks fine to me :) Your turn, by the way!

[felipecao]

Hey, @tobias-reuter , our code is looking good! It seems to me like we're getting pretty close to the point where we won't be able to come up with further failing scenarios. Which is also nice, because allows us to do further refactoring. One refactoring we could already do is extracting the "production" code from the test class into a separate Java class. If you feel like doing it, feel free to do it :)

I was also putting some thought on other possible improvements, and I thought we could also use this kata to practice the concepts presented in https://www.cs.helsinki.fi/u/luontola/tdd-2009/ext/ObjectCalisthenics.pdf . For instance, I have some ideas about how to get rid of the IF's we currently use to decide when to print "Fizz" or "Buzz", but I can wait until we have a good coverage on the "FizzBuzz" cases.

What do you think about pushing our small kata further into that direction?

[tobias-reuter]

@felipecao, just pushed the refactoring. I was using Refactor / Delegate to extract into a separate class. But I had to move the new class into the main path by myself. Could have been more comfortable.

I installed Infinitest plugin and activated Compiler / Make project automatically. Every change I make will now trigger a test run. You should try it.

I'm no sure if this kata has enough meat to apply the object calisthenics, but let's try it!

[felipecao]

Hey, @tobias-reuter, as you may see from the last push, I've changed a lot of stuff. I'll try to explain what I had in mind, I hope I can make myself clear enough to convey my ideas.

So, what I wanted to do was moving the IF's we had on FizzBuzz class to some other class. Besides, I wanted to implement calisthenic #3 (wrap all primitives and Strings) and make FizzBuzz logic simpler.

So, what I did first was implementing the concept of Number. So, instead of us dealing with primitive integers like 3 and 5, we'd rely on instances of Number interface: FizzNumber, BuzzNumber, FizzBuzzNumber and RegularNumber. These classes would be responsible for knowing what to output: either "Fizz", "Buzz", "FizzBuzz" or the string representation of the actual number.

But I didn't want to refactor FizzBuzz class so that it would have to decide which Number it should instantiate, I wanted to make FizzBuzz as simple and easily readable as possible. So, I created Numbers class, which would take our upper boundary input and figure out the conversion of the upper boundary to a collection of numbers, depending on the module operations with 3 and 5.

But then I realized Numbers had too many responsibilities: it was being responsible for instantiating the collection of numbers and also deciding which instance to use based on the input.

So I moved the last part to NumberFactory, which would take an input and decided whether to return a FizzNumber, BuzzNumber, FizzBuzzNumber or a RegularNumber.

After all of this was done, refactoring FizzBuzz was a piece of cake, it just had to rely on Numbers to do all the hard work. FizzBuzz just had to retrieve each number's string representation and output it.

Have I been able to properly explain the ideas? I always suck at that. And I also think it might have been too many steps at once, and I might have made it quite hard for you to follow the changes, I apologize if that's the case.

But there still are improvements to do.

For instance, we can still improve our code to comply with calisthenic # 1 (one level of indentation), I know for a fact that there still are methods that violate this principle. And I'd love to get rid of the math operations we moved from FizzBuzz to NumberFactory. If you can think of a way to use some alternative to those calculations (can we use polymorphism for that, somehow?), that'd be awesome!

Let me know your thoughts and how you think we can further improve this. I think there might still be some juice for us to squeeze out of this. If you feel like I did too much and want to start a new kata, I'm up for that too! :)

Cheers!

[tobias-reuter]

Hi @felipecao, looking forward to look into detail what you did and to do the next step. I'm sorry that I'm not so responsive. I hope it will be better the next days.

I just took some time to comment on your commit: https://github.com/felipecao/coding-katas/commit/55c82e1050040360bcbec182bc4a39dac4f08519

Let me know what you think about it.

Bye

[felipecao]

Hey, @tobias-reuter, no worries about responsiveness, take your time :)

BTW, due to security policies in my company I had to enable 2FA authentication in my github account, I hope this doesn't prevent you from pulling or pushing any code.

I've updated the code according to your comment, btw :)

Cheers!

[tobias-reuter]

Hey @felipecao ,

I finally could review in more detail what you have done so far. Wow, it looks like you spend some time on refactoring.

Let’s go through the checklist

• [x] 1. One level of indentation per method I think that is done. I couldn’t find more than two (?) levels. But that ok according to the Object Calisthenics document.

void collectRows(StringBuffer buf) {
    for (int i = 0; i < 10; i++)
        collectRow(buf, i);
}

• [x] 2. Don’t use the ELSE keyword passing.
• [x] 3. Wrap all primitives and Strings With fizz.buzz.number.Number you wrapped the FizzBuzz numbers. Done.
• [ ] 4. First class collections I think this is still open. fizz.buzz.number.NumbersImpl#fetch doesn't return a first class collection. That first class collection could actually implement toString() and reduce fizz.buzz.FizzBuzz#fizzBuzz further.
• [x] 5. One dot per line We have some minor violation as in fizz.buzz/number/AbstractNumber.java:35

return value.equals(that.value);

But I think that is fine.

• [ ] 6. Don’t abbreviate Violations: NumberFactoryImpl -> NumberFactoryImplementation NumbersImpl -> NumbersImplementation
• [x] 7. Keep all entities small "This means no class over 50 lines" I think they are.
• [x] 8. No classes with more than two instance variables passing.
• [x] 9. No getters/setters/properties passing.

[tobias-reuter]

@felipecao don't touch the factory. I'm about to get rid of the if cascade. ;-)

[tobias-reuter]

just created a pull request #3 .

I isn't finished yet, I just wanted to get your input before I continue.

[felipecao]

Hey, @tobias-reuter , thx for the feedback! Some comments on top of those:

• About items 4 & 6, you're absolutely right, I'll think about how to fix 4 and will rename the classes afterwards (even though I personally like the Impl suffix better :) ).
• About item 5, the point in Object Calisthenics about the dots is to prevent violating Demeter's Law. In this case, since we're talking about a comparison between the same attribute within the same class, I agree with you, I think we're fine .
• About the 50 lines threshold, I checked all of them and we're fine :)

About the pull request, I liked your idea and did something on top of it on https://github.com/felipecao/coding-katas/pull/4. I've extended your initial idea and applied the "Chain of Responsibility" design pattern. Let me know what do you think about it!

Cheers!

[tobias-reuter]

It takes some time but I have to say that I like doing it. Searching for good solutions, trying to keep the code clean.

I have two more ideas in my mind, but I'm running out of time for today.