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

Circular Wrapping Text, minor adjustments

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

Problem

This basically it sets a shapes default size and radius based on how many characters are in a string that is passed to it, the user gets the option to manually set the Font, otherwise it is automatically set for them. All letters are set to face towards the center of the circle as it wraps around. I do seem to have two minor flaws in the algorithm, the circular text is slightly offset from the circle object to the right I think and so I have to manually adjust the padding around the sides based on the size of the circle generated.

Another flaw is that as more text is placed in the circle the beginning and end doesn't have consistent spacing with the rest of the sentence, it has a bit of an extra gap which seems to get slightly bigger as more characters are placed in the circle.

If you look at the picture on the right, just noticed this, some of the letters are slightly offset just by a few pixels:

Now I was only required to manually enter the positions of each letter, but I decided to go above and beyond so I would have reusable code for future projects.

```
/** Main */
import javafx.application.Application;
import javafx.geometry.Insets;
import javafx.geometry.Pos;
import javafx.scene.Scene;
import javafx.scene.layout.GridPane;
import javafx.stage.Stage;

/**
* Created by John on 7/11/2014.
*/
public class Prog14_05 extends Application {
@Override
public void start(Stage primaryStage) {
// Create Pane
circularText phrase = new circularText("WE ARE ANONYMOUS ",
480, 480);
phrase.setFont("Matrix");
phrase.setTextSize(30);

// Place clock and label in border pane
GridPane pane = new GridPane();
pane.setAlignment(Pos.CENTER);
pane.setPadding(new Insets(15, 30, 30, 0));
pane.setStyle("-fx-background-color: black");
pane.getChildren().add(phrase);

// Create a scene and place it in the stage
Scene scene = new Scene(pane);
primaryStage.setT

Solution

Naming

Your class is named circularText, it should be named CircularText as all classes in Java should start with a capital character.

Declared fields

You have declared two fields as private but what about the rest?

double textSize = 30;
String string = "";
String fontName = "";
Font font = new Font("Times Roman", textSize);


All these should be marked private. In addition, string should be marked final and does not need to be initialized there as it gets a new value in the constructor.

Your textSize gets a value in the constructor so the value 30 is only used for initializing the font. This may lead to potential bugs. Your font variable should also be initialized in the constructor, after the textSize value.

Your String string; has a horrible name. I can see that it is a string, what is it used for? A better name would be text or phrase.

Width and height fields

// Pane's width and height
private double w = 250, h = 250;


  • Please only declare one field per line, it gets easier to read that way.



  • Instead of adding a comment // Pane's width and height, why not name the variables as width and height directly? Then you don't need that comment. Comments should describe why, not the what.



  • These variables also get a value in the constructor so initializing them to 250 here is useless.



  • The getters and setters for these are just as badly named as the variables themselves, getWidth/setHeight/etc. would be a lot better names for those methods.



Circle, what circle?

Circle circle = new Circle(centerX, centerY, clockRadius);
circle.setFill(null);
circle.setStroke(null);
getChildren().add(circle);


I'm not sure what purpose your Circle fills, I tried removing this code and I did not see any change. You also don't provide any way to provide any paint to the circle, and when I tried adding some color to the circle it's probably a good thing that you didn't.

Char to String

phrase.charAt(i) + "" is a dirty way of converting a char to a string. A better method would be String.valueOf(phrase.charAt(i)).

Double precision

double degree = 360 / phrase.length();


Even though you store the result as a double, you're actually using int division here which makes the result an int. To add some accuracy, you have to specify one of the ints as being a double, a simple way to do this is:

double degree = 360.0 / phrase.length();


Missing flexibility

You provide some ways of altering the text, but you do not provide a way to change this:

letter.setFill(Color.LIME);


Or this:

letter.setRotate(degrees + 90);


Summary

It is a nice component you are building here that I see a lot of use-cases for, but after you've fixed the bugs with the positioning, please provide some more flexibility for users of your component to modify more aspects of it. I like a lot that you are using JavaFX.

Code Snippets

double textSize = 30;
String string = "";
String fontName = "";
Font font = new Font("Times Roman", textSize);
// Pane's width and height
private double w = 250, h = 250;
Circle circle = new Circle(centerX, centerY, clockRadius);
circle.setFill(null);
circle.setStroke(null);
getChildren().add(circle);
double degree = 360 / phrase.length();
double degree = 360.0 / phrase.length();

Context

StackExchange Code Review Q#56956, answer score: 7

Revisions (0)

No revisions yet.