patternjavaMinor
Dynamically Generating a Map Webpage for MMO
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
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:
What you should be using is h1 (headline):
Does your legend really need both lines of text? Simplify (and use the appropriate markup):
The owners themselves should be a list:
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):
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.
If you're not reusing that information in both places, I'd suggest using :nth-child rather than a class (or id):
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 MapDoes your legend really need both lines of text? Simplify (and use the appropriate markup):
Region OwnersThe 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.