4
\$\begingroup\$

This question is related to my previous question about an MMO Server.

Here, what I am doing with this program is connecting to another Java server program that resides on the same server as the MMO server. Once connected it receives a payload that contains all of the player data for the MMO server as well as all of the region data. This is about 15 megabytes of data, and the transfer happens every hour or so.

With this data, I create a simple webpage. What I will be posting in this question is the code used to generate the page. I know I am doing lots of things wrong, so hopefully there will be a lot to criticize and for me to improve.

The region data is analyzed and then the image for the map on the web page is created based on that data.

Here is a link to the live web page:

World Map

public class RegionWebsiteGenerator {

    private Map<MapPoint, String> regionMap = new HashMap<MapPoint, String>();
    private Map<String, Integer> regionOwners = new HashMap<String, Integer>();
    private Map<String, Integer> topOwnersAndRanks = new HashMap<String, Integer>();

    private int regionWidth = 12;
    private int numRegions = 60; //need to know this number ahead of time

    public RegionWebsiteGenerator() {
        this.loadRegions();
        this.countOwnersForRegions(this.regionMap);
        this.populateTopOwners(this.regionOwners);
        this.createMapImageForRegions(this.regionMap);
    }

    //load and parse data
    private void loadRegions() {
        File dir = new File("regions");
        File[] directoryListing = dir.listFiles();
        if (directoryListing != null) {
            for (File child : directoryListing) {

                //validation
                String fileName = child.getName();
                String delims = ","; //separator for map point
                String[] points = fileName.split(delims);
                if (points.length > 0) {
                    try {
                        MapPoint point = new MapPoint(Integer.parseInt(points[0]), Integer.parseInt(points[1]));

                        //read the valid file
                        if (point != null) {
                            BufferedReader br = new BufferedReader(new FileReader(child));
                            @SuppressWarnings("unused")
                            String firstLine = br.readLine();
                            String secondLine = br.readLine(); //second line is the region data
                            this.regionMap.put(point, secondLine);
                            br.close();
                        }
                    } catch (Exception e) {
                        System.out.println("file is not a region");
                    }
                }   
            }
        }
    }   
    private void countOwnersForRegions(Map<MapPoint, String>regions) {
        for (String regionString : regions.values()) {
            String ownerName = this.getOwnerNameForRegion(regionString);
            if (this.regionOwners.containsKey(ownerName)) {
                int currentCount = this.regionOwners.get(ownerName);
                this.regionOwners.put(ownerName, currentCount++);
            } else {
                this.regionOwners.put(ownerName, 1);
            }
        }
    }
    private void populateTopOwners(Map<String, Integer>regionOwners) {
        Set<Entry<String, Integer>> entries = PlayerWebsiteGenerator.entriesSortedByValues(regionOwners);
        Object[] entriesArray = entries.toArray();
        int maxCount = Math.min(entriesArray.length, 10);
        for (int i = 0; i < maxCount; i++) {
            @SuppressWarnings("rawtypes")
            Entry entry = (Entry)entriesArray[i];
            String name = (String)entry.getKey();
            if (!name.equals("^")) {
                this.topOwnersAndRanks.put((String)entry.getKey(), i);
            }
        }
    }

    //output the html
    public void generatePage() {
        String filePath = "../www/site1/stats/regions.html";
        File regionHTMLFile = new File(filePath);
        try {
            regionHTMLFile.createNewFile();

            BufferedWriter output;
            try {
                output = new BufferedWriter(new FileWriter(regionHTMLFile));
                output.write("<!doctype html>");
                output.newLine();
                output.write("<html>");
                output.newLine();
                output.write("<link rel=\"stylesheet\" type=\"text/css\" href=\"regions.css\" media=\"screen\" />");
                output.newLine();
                output.write("<head>");
                output.newLine();
                output.write("</head>");
                output.newLine();
                output.write("<body onload=\"javascript:scrollWin()\">");
                output.newLine();

                output.write("<div id=\"title\">");
                output.newLine();
                output.write("<p>Live Region Map</p>");
                output.newLine();
                output.write("</div>");
                output.newLine();

                output.write("<div id=\"map\">");
                output.newLine();
                output.write("<img src=\"regions.png\" alt=\"Map\">"); //style=\"width:1800px;height:1800px\"
                output.newLine();
                output.write("</div>");
                output.newLine();

                output.write("<div id=\"legend\">");
                output.newLine();
                output.write("<p>Region Owners</p>");
                output.newLine();
                output.write("<p>Legend</p>");
                output.newLine();

                this.writeLegendToHTML(output);

                output.write("</div>");
                output.newLine();

                output.write("<script>");
                output.newLine();
                output.write("function scrollWin() {");
                output.newLine();
                output.write("window.scrollTo(window.outerHeight/2, window.outerWidth/2);");
                output.newLine();
                output.write("}");
                output.newLine();
                output.write("</script>");
                output.newLine();

                output.write("</body>");
                output.newLine();
                output.write("</html>");
                output.close();
            } catch (IOException e) {
                e.printStackTrace();
            }    
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
    private void writeLegendToHTML(BufferedWriter output) throws IOException {
        int currentRank = 0;
        for (String string : this.topOwnersAndRanks.keySet()) {
            output.write("<div id=\"rank" + String.valueOf(currentRank) + "\">");
            output.newLine();
            output.write("<p>" + string + "</p>");
            output.newLine();
            output.write("</div>");
            output.newLine();
            currentRank++;
        }
    }

    //generate the map image
    private void createMapImageForRegions(Map<MapPoint, String>regions) {

        BufferedImage img = new BufferedImage(numRegions*regionWidth, numRegions*regionWidth, BufferedImage.TYPE_INT_RGB);

        for (MapPoint point : regions.keySet()) {

            int i = point.x * regionWidth;
            int j = (59 - point.y) * regionWidth; //flip the image because it draws starting top left

            String ownerName = this.getOwnerNameForRegion(regions.get(point));

            Color color = new Color(this.getColorForBiomeType(this.getBiomeTypeForRegion(regions.get(point))));
            if (this.topOwnersAndRanks.containsKey(ownerName)) {
                color = new Color(this.getColorForRank(this.topOwnersAndRanks.get(ownerName)));
            }

            Graphics graphics = img.getGraphics();
            graphics.setColor(color);
            graphics.drawRect(i, j, regionWidth, regionWidth);
            graphics.fillRect(i, j, regionWidth, regionWidth);
        }

        File imageFile = new File("../www/site1/stats/regions.png");
        try {
            ImageIO.write(img, "PNG", imageFile);
        } catch (IOException e) {
            System.out.println("failed to write image");
        }
    }
    private int getColorForRank(int rank) {
        int r = 0;
        int g = 0;
        int b = 0;
        int a = 255;
        switch (rank) {
            case 0:
                //gold
                r = 204;
                g = 204;
                b = 0;
                break;
            case 1:
                //Dark magenta
                r = 153;
                g = 0;
                b = 76;
                break;
            case 2:
                //dark red
                r = 153;
                g = 0;
                b = 0;
                break;
            case 3:
                //light blue
                r = 153;
                g = 255;
                b = 255;
                break;
            case 4:
                //pink
                r = 255;
                g = 204;
                b = 204;
                break;
            case 5:
                //light gray
                r = 224;
                g = 224;
                b = 224;
                break;
            case 6:
                //dark gray
                r = 64;
                g = 64;
                b = 64;
                break;
            case 7:
                //dark blue
                r = 0;
                g = 0;
                b = 102;
                break;
            case 8:
                //dark purple
                r = 51;
                g = 0;
                b = 102;
                break;
            case 9:
                //light purple
                r = 153;
                g = 153;
                b = 255;
                break;
            default:
                //will be black
                break;
        }
        //color is created by this bit shifting trick
        return (a << 24) | (r << 16) | (g << 8) | b;
    }
    private int getColorForBiomeType(int biomeType) {
        int r = 0;
        int g = 0;
        int b = 0;
        int a = 255;
        if (biomeType == 0) {
            //woodlands
            r = 52;
            g = 108;
            b = 0;
        } else if (biomeType == 1) {
            //mountains
            r = 102;
            g = 51;
            b = 0;
        } else if (biomeType == 2) {
            //plains
            r = 0;
            g = 255;
            b = 0;
        } else if (biomeType == 3) {
            //wasteland
            r = 255;
            g = 153;
            b = 51;
        } else if (biomeType == 4) {
            //swampy
            r = 0;
            g = 153;
            b = 76;
        } else if (biomeType == 5) {
            //aquatic
            r = 0;
            g = 128;
            b = 255;
        }
        //color is created by this bit shifting trick
        return (a << 24) | (r << 16) | (g << 8) | b;
    }

    //get the necessary information using the pattern found in deserialization method below
    private int getBiomeTypeForRegion(String region) {
        String delims = DelimiterType.TOP_LEVEL.string;
        String[] messageFragments = region.split(delims);
        return Integer.parseInt(messageFragments[0]);
    }
    private String getOwnerNameForRegion(String region) {
        String delims = DelimiterType.TOP_LEVEL.string;
        String[] messageFragments = region.split(delims);
        return messageFragments[4];
    }
    /*
    public static Region decodeRegionFromString(String string) {
        String delims = DelimiterType.TOP_LEVEL.string;
        String[] messageFragments = string.split(delims);

        int biomeType = Integer.parseInt(messageFragments[0]);
        Region region = new Region(BiomeType.values()[biomeType], new MapPoint(Integer.parseInt(messageFragments[1]), Integer.parseInt(messageFragments[2])));

        region.name = messageFragments[3];
        region.setOwnerName(messageFragments[4]);

        int numTiles = Integer.parseInt(messageFragments[5]);
        int messageIndex = 6;
        for (int i = 0; i < numTiles; i++) {
            int tileType = Integer.parseInt(messageFragments[messageIndex]);
            messageIndex++;

            if (tileType == TileType.CAPITAL.ordinal()) {
                TileCapital tile = TileCapital.decodeTileFromString(messageFragments[messageIndex]);
                messageIndex++;
                region.setTileForPosition(tile.position(), tile);
            } else {
                Tile tile = Tile.decodeTileFromString(TileType.values()[tileType], messageFragments[messageIndex]);
                messageIndex++;
                region.setTileForPosition(tile.position(), tile);
            }
        }
        return region;
    }
    */
    //DO NOT DELETE COMMENTED METHOD
}

Here is the generated HTML file:

<!doctype html>
<html>
<link rel="stylesheet" type="text/css" href="regions.css" media="screen" />
<head>
</head>
<body onload="javascript:scrollWin()">
<div id="title">
<p>Live Region Map</p>
</div>
<div id="map">
<img src="regions.png" alt="Map">
</div>
<div id="legend">
<p>Region Owners</p>
<p>Legend</p>
<div id="rank0">
<p>Name14</p>
</div>
</div>
<script>
function scrollWin() {
window.scrollTo(window.outerHeight/2, window.outerWidth/2);
}
</script>
</body>
</html>

And here is the CSS file which currently never changes:

body {
    background-color:black;
}

#title {
    font-size: 40pt;
    color: white;
    text-align: center;
    border:5px solid white;
}

#map {
    text-align: center;
}

#legend {
    border: 5px solid white;
    text-align: center;
    font-size: 30pt;
    color: white;
}

#rank0 {
    text-align: center;
    background-color: rgb(204, 204, 0);
}
#rank1 {
    text-align: center;
    background-color: rgb(153, 0, 76);
}
#rank2 {
    text-align: center;
    background-color: rgb(153, 0, 0);
}
#rank3 {
    text-align: center;
    background-color: rgb(153, 255, 255);
}
#rank4 {
        text-align: center;
    background-color: rgb(255, 204, 204);
}
#rank5 {
    text-align: center;
    background-color: rgb(224, 224, 224);
}
#rank6 {
    text-align: center;
    background-color: rgb(64, 64, 64);
}
#rank7 {
    text-align: center;
    background-color: rgb(0, 0, 102);
}
#rank8 {
    text-align: center;
    background-color: rgb(51, 0, 102);
}
#rank9 {
    text-align: center;
    background-color: rgb(153, 153, 255);
}

I know very little about HTML and CSS so any comments on those would also be appreciated.

\$\endgroup\$
  • \$\begingroup\$ There is no need for new-line characters in HTML or JavaScript. You can safely remove them. \$\endgroup\$ – GiantTree May 8 '15 at 17:25
4
\$\begingroup\$

Color

private int getColorForRank(int rank) {
        int r = 0;
        int g = 0;
        int b = 0;
        int a = 255;
        switch (rank) {
            case 0:
                //gold
                r = 204;
                g = 204;
                b = 0;
                break;
            case 1:
                //Dark magenta
                r = 153;
                g = 0;
                b = 76;
                break;
            case 2:
                //dark red
                r = 153;
                g = 0;
                b = 0;
                break;
            case 3:
                //light blue
                r = 153;
                g = 255;
                b = 255;
                break;
            case 4:
                //pink
                r = 255;
                g = 204;
                b = 204;
                break;
            case 5:
                //light gray
                r = 224;
                g = 224;
                b = 224;
                break;
            case 6:
                //dark gray
                r = 64;
                g = 64;
                b = 64;
                break;
            case 7:
                //dark blue
                r = 0;
                g = 0;
                b = 102;
                break;
            case 8:
                //dark purple
                r = 51;
                g = 0;
                b = 102;
                break;
            case 9:
                //light purple
                r = 153;
                g = 153;
                b = 255;
                break;
            default:
                //will be black
                break;
        }
        //color is created by this bit shifting trick
        return (a << 24) | (r << 16) | (g << 8) | b;
    }

I would define an Enum Color like so :

public enum Color {
    GOLD(204,204,0);

    private int r = 0;
    private int g = 0;
    private int b = 0;
    private int a = 255;

    private Color(int r, int g, int b) {
        this.r = r;
        this.g = g;
        this.b = b;
    }

    public int color() {
        return (a << 24) | (r << 16) | (g << 8) | b; 
    }
}

And now, the only thing you need to do is constructing a Map of colors and you can just do something like mapOfColors.get(1).color() and voila! You can rince and repeat for every color you have (biome and etc.)

SuppressWarnings are a code smell

Most of the times @Suprresswarnigns is a code smell. Normally you can always do something to avoid the issue at hand or it's warning you that you're doing something bad! Let's look at your issue :

                        @SuppressWarnings("unused")
                        String firstLine = br.readLine();

This is warning you that you don't use firstLine. If you don't need it, why do you create a variable and assigned it. You know you can ignored returned value.

 br.readLine();//skip the first line

Templating

What you're doing by hand could easily be done by using a templating engine like Velocity, or transforming this in a webapp. It would simplify the generatePage() since you would construct your model variable with all the information you need and just publish the page.

Writing a small Java web app for one page could look overkilled, but I'm sure you will add some pages in the future, and I hope by that time you will not generate all of them by doing what you are currently doing.

Trivia

private int regionWidth = 12;

This is a constant so it should be :

private final static int REGION_WIDTH = 12;
//DO NOT DELETE COMMENTED METHOD

Commented method are just noise in your class. I hope you're using a source control, if you do delete that code and just make a tag to remember where to find it, but as it stand it's not useful at all.

\$\endgroup\$
4
\$\begingroup\$

HTML in your Java

I don't know anything about Java, but that amount of static markup hard-coded like that with print statements raises a red flag for me. Aren't there functions for reading the contents of files that you can use? Seems like that would would make it easier to make modifications later when your template changes.

Use the most semantically appropriate markup

You're using the following markup for your page title:

<div id="title">
<p>Live Region Map</p>
</div>

What you should be using is h1 (headline):

<h1>Live Region Map</h1>

Does your legend really need both lines of text? Simplify (and use the appropriate markup):

<h2>Region Owners</h2>

The owners themselves should be a list:

<ul class="legend">
<li>baz</li>
<li>Krepi</li>
</ul>

Overusing IDs

I don't really see a need to use IDs here at all. You're not using them as hooks for JavaScript. If you're not sure whether you should use a class or id, go with a class.

Take advantage of the cascade

For your legend, you're specifying the text-align 10 times! Just once will do (using the markup from above):

.legend li {
    text-align: center;
}

Inline vs Linked CSS

Normally, I'd be advising anyone who is using line styles to switch to placing that information in an external stylesheet. However, with dynamically generated content like this, especially when you're reusing that same information to generate that image, it might be better to have it located in place (in your Java) and use inline styles.

<ul class="legend">
<li style="background-color: rgb(204, 204, 0)">baz</li>
<li style="background-color: rgb(153, 0, 0)">Krepi</li>
</ul>

If you're not reusing that information in both places, I'd suggest using :nth-child rather than a class (or id):

.legend li:nth-child(1) {
    background-color: rgb(204, 204, 0);
}
.legend li:nth-child(2) {
   background-color: rgb(153, 0, 76);
}
.legend li:nth-child(3) {
   background-color: rgb(153, 0, 0);
}
.legend li:nth-child(4) {
   background-color: rgb(153, 255, 255);
}
.legend li:nth-child(5) {
    background-color: rgb(255, 204, 204);
}
.legend li:nth-child(6) {
   background-color: rgb(224, 224, 224);
}
.legend li:nth-child(7) {
   background-color: rgb(64, 64, 64);
}
.legend li:nth-child(8) {
   background-color: rgb(0, 0, 102);
}
.legend li:nth-child(9) {
   background-color: rgb(51, 0, 102);
}
.legend li:nth-child(10) {
   background-color: rgb(153, 153, 255);
}
\$\endgroup\$
  • \$\begingroup\$ Your answer was very helpful and enlightening. Much appreciated! \$\endgroup\$ – bazola May 11 '15 at 22:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.