Creating pop up dialog window

I've decided to make this module of a dialog, sort of like talking to NPCs in games, like this for example.

I've creating the visuals of the window using the code from this post, works pretty well.

Here's an example:

I've created a Manager class to manage those pop-ups, and the obvious PopUpDialog class.

Here's the Manager class:

```
public class PopUpDialogManager
{
#region Singleton

///
/// .Net Lazy object for singleton
///
private static readonly Lazy Lazy =
new Lazy(() => new PopUpDialogManager());

///
/// Returns the singleton instance of the ScreenManager
///
public static PopUpDialogManager Instance
{
get { return Lazy.Value; }
}

///
/// Constructs the PopUpDialogManager
///
private PopUpDialogManager()
{

}

#endregion

public GraphicsDevice GraphicsDevice { get; private set; }
public SpriteFont Font { get; private set; }
public SpriteFont TitleFont { get; private set; }
private PopUpDialog CurrentDialog { get; set; }

public Texture2D GetTexture(int width, int height)
{
return RectangleGenerator.CreateRoundedRectangleTexture(
graphics: GraphicsDevice,
width: width,
height: height,
borderThickness: 2,
borderRadius: 4,
borderShadow: 2,
backgroundColors: new List { Color.AntiqueWhite },
borderColors: new List { Color.Azure },
initialShadowIntensity: 0.4f,
finalShadowIntensity: 0.2f);
}

public void CloseDialog()
{
CurrentDialog = null;
}

public void LoadContent(ContentManager content, GraphicsDevice graphicsDevice)
{
GraphicsDevice = graphicsDevice;
Font = content.Load("Fonts/Font");
TitleFont = content.Load("Fonts/TitleFont");
}

public void CreateDialog(string title,
IEnumerable messages,

PopUpDialog

• The name ShouldContinueScrolling doesn't read right IMO and should be changed to ShouldScrollingContinue.

-
ApplyPositionOption() is a little bit over the top. You only have two members for PositionEnum so a simple if with a future else will be enough and reduces the horizontal spacing which makes the code more readable.

-
string SetFirstDisplayMessage() why does a method prefixed with Set returns a value ? This should be named GetFirstDisplayMessage() instead.

-
SetTextBorderSize() doesn't do what the name implies. Either you create the textRectangle somewhere else and use the titleRectangle to adjust the size of it or you should change the name. The same is true for SetTitleBorderSize().

Clarification based on the comment


The SetXBorderSize methods tho, I don't really get what's the problem with them.

If I read SetTextBorderSize or SetTitleBorderSize I expect the method doing just setting a size. In these methods the Rectangles aren't adjusted by setting Width, Height etc but by using the constructor of the struct. If you name that method CreateTextBorder and let it return the created Rectangle it would be more obvious.

-
ApplyResizeOption() has the same "problems" like ApplyPositionOption()

-
GetFittingTitle() seeing string concatination inside a loop is IMO a red sign. If I see something like this the first what comes to my mind is use a StringBuilder.

General

-
don't omit braces {} although they might be optional. This helps to make your code less error prone.

-
if you have choosen a coding style ( like omiting/having braces ) you should stick to that style. Right now you are mixing styles, for instance sometimes you use braces sometimes you don't.

-
adding vertical space (new lines) to group related code will lead to better readability and is therfor better to maintain.

StackExchange Code Review Q#108713, answer score: 3