HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Dynamically Generating a Map Webpage for MMO

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mapdynamicallywebpagegeneratingformmo

Problem

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 regionMap = new HashMap();
private Map regionOwners = new HashMap();
private Map topOwnersAndRanks = new HashMap();

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

Solution

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:


Live Region Map


What you should be using is h1 (headline):

Live Region Map


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

Region Owners


The owners themselves should be a list:


baz
Krepi


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.


baz
Krepi


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);
}

Code Snippets

<div id="title">
<p>Live Region Map</p>
</div>
<h1>Live Region Map</h1>
<h2>Region Owners</h2>
<ul class="legend">
<li>baz</li>
<li>Krepi</li>
</ul>
.legend li {
    text-align: center;
}

Context

StackExchange Code Review Q#90186, answer score: 4

Revisions (0)

No revisions yet.