Over a million developers have joined DZone.
{{announcement.body}}
{{announcement.title}}

Real Coding: Cleaning Things Up

DZone's Guide to

Real Coding: Cleaning Things Up

One day you turn the system upside down, the other you clean things up so that further progress is possible. Today's the other day, and here comes the cleaning!

· Java Zone
Free Resource

Learn how to troubleshoot and diagnose some of the most common performance issues in Java today. Brought to you in partnership with AppDynamics.

Welcome to the fourth installment of my Real Coding series. In this part, we'll face the mess that we've made during the previous Pomodoro and make ourselves ready to implement the rest of our application's business logic. Let's dive straight into it!

Moving Code Around

The first thing that I purposefully ignored at the beginning (and that has started annoying me already), was that I kept all of the code in a single source file. When you're just "sketching out" the initial design, creating a separate file for every single class is a waste of time, as classes are created and deleted very often in the process.

Now, as the design has matured a little bit (I hope), we can start extracting bits and pieces into separate files to avoid unnecessary distractions and an overall feeling of messiness. A good starting point is a single source file for each of the aggregates with their related classes and a separate file for the only service that we've got.

Image title

At a later stage, when the aggregates themselves grow, these files might be further split into separate packages. I won't do this now, as I see no need to do so. Decisions like this one are easy to change later, so it's better not to overthink and just stick with what feels right at the moment.

Unfinished Business

In the previous part, as I split a huge Ranking aggregate into a few smaller aggregates, I wrote a piece of code like this:

class Ranking(val name: String,
              val defaultRating: Int,
              val ratings: Set<Rating> = mutableSetOf(),
              val matches: MutableList<Match> = mutableListOf()) {

    // irrelevant stuff has been cut off

    fun isRanked(player: Player) = ratings.any { it.player == player }
}

class Match(val dateTime: LocalDateTime,
            val player1: Player,
            val player2: Player,
            val ranking: Ranking,
            var result: MatchResult,
            var status: MatchStatus = MatchStatus.WAITING_FOR_PLAYER2,
            val id: MatchId = MatchId()) {

    init {
        requireRanked(player1, ranking)
        requireRanked(player2, ranking)
    }

    private fun requireRanked(player: Player, ranking: Ranking) {
        if (!ranking.isRanked(player))
            throw PlayerNotRankedException(player)
    }
}


As you can see, there's a "business" rule here that says that we cannot create a match if any of the players have not joined a given ranking. That decision was taken in Part 2, and I'd like to stick to it for now.

The problem with this piece of code is the cyclic dependency between the rating and the ranking, which proved annoying when trying to update the tests according to the redesign. I've decided to tackle this problem by moving the check into a factory, which will base the decision based on the ratings rather than the ranking object itself:

class MatchFactory(val ratings: RatingRepository) {

    fun create(dateTime: LocalDateTime, player1: Player, player2: Player, ranking: Ranking, result: MatchResult): Match {
        requireRanked(player1, ranking)
        requireRanked(player2, ranking)
        return Match(dateTime, player1, player2, ranking, result)
    }

    private fun requireRanked(player: Player, ranking: Ranking) {
        if (!ratings.existsByPlayerAndRanking(player, ranking)) {
            throw PlayerNotRankedException(player)
        }
    }
}


This way, if I want to test any method that requires a player to join a ranking, I can simply create the necessary rating instead of also updating a related ranking object.

The Green Bar Is Back!

Now that I satisfied my desire to move things around and the testability has been improved, I got a chance to get the tests working again so that the project is fully on the right track.

The path to the Green Bar Heaven was relatively easy, as it just required me to instantiate the service with all necessary dependencies and invoke the service instead of the previously refactored aggregate:

class RankingServiceTest {
    private val matchTime = LocalDateTime.of(2017, 8, 18, 21, 0)
    private val grzegorz = Player("grzegorz")
    private val james = Player("james")
    private val ranking = Ranking("DZone Smackdown?!", 1200)

    private val ratingRepo = InMemoryRatingRepository()
    private val playerRepo = InMemoryPlayerRepository()
    private val rankingRepo = InMemoryRankingRepository()
    private val matchRepo = InMemoryMatchRepository()
    private val matchFactory = MatchFactory(ratingRepo)

    private val service = RankingService(ratingRepo, playerRepo, rankingRepo, matchRepo, matchFactory)

    @BeforeEach
    fun setUp() {
        playerRepo.players.add(grzegorz)
        playerRepo.players.add(james)
        rankingRepo.rankings.add(ranking)
    }

    @Test
    fun cannotPlayWithoutJoining() {
        assertThrows(PlayerNotRankedException::class.java) {
            service.addMatch(matchTime, grzegorz.name, james.name, PLAYER1_WON.toString(), ranking.name)
        }
    }

    @Test
    fun canPlayAfterJoining() {
        service.join(grzegorz.name, ranking.name)
        service.join(james.name, ranking.name)

        service.addMatch(matchTime, grzegorz.name, james.name, PLAYER1_WON.toString(), ranking.name)

        assertEquals(
                setOf(Match(matchTime, grzegorz, james, ranking, PLAYER1_WON)),
                matchRepo.matches)
    }
}


There are a few smells here, which I'm not yet sure I'll act upon:

  • The service requires quite a few dependencies to do its job, and it's not settled that these are all that will be ever needed.

  • The in-memory repositories would probably benefit from some setup method so that I don't access their underlying collections directly.

  • The second test asserts match equality, which I'm not sure is a good idea. Semantically, I'd rather check that "a match with provided parameters has been added", rather than "that specific match has been added".

Anyways, the code above will suffice for now and I'm happy to be back in the green.

Next Steps

At this point, I ran out of time for this Pomodoro, so here ends the article. The code appears to be clean enough to proceed and implement the rest of the application's use cases. The challenge for next time? Implement and test all of the remaining business logic for solo player games, so that I can finally have a working piece of software.

Understand the needs and benefits around implementing the right monitoring solution for a growing containerized market. Brought to you in partnership with AppDynamics.

Topics:
package structure ,testability ,factory pattern ,java ,kotlin ,tutorial

Opinions expressed by DZone contributors are their own.

{{ parent.title || parent.header.title}}

{{ parent.tldr }}

{{ parent.urlSource.name }}