Address code review comments

* Capitalise Percent and Pixel enums and resource keys
* Remove commented code
* Rewrite PixelPercent constructors to use int.TryParse
* Rewrite PixelPercent.CanParse

Add tests for PixelPercent
This commit is contained in:
David Gardiner 2016-02-29 20:59:05 +10:30
parent a25c622726
commit 8798073e82
10 changed files with 176 additions and 39 deletions

View File

@ -3124,6 +3124,10 @@ namespace OpenLiveWriter.Localization
/// </summary>
PasteSpecialThinnedLabel,
/// <summary>
/// Percent (unit of measure)
/// </summary>
Percent,
/// <summary>
/// Ping Servers
/// </summary>
PingPrefName,
@ -3132,9 +3136,9 @@ namespace OpenLiveWriter.Localization
/// </summary>
PingPrefUrl,
/// <summary>
/// pixels
/// Pixels (unit of measure)
/// </summary>
pixels,
Pixels,
/// <summary>
/// &Map
/// </summary>
@ -5169,11 +5173,7 @@ namespace OpenLiveWriter.Localization
/// <summary>
/// There was an unexpected error while uploading the video.
/// </summary>
YouTubeVideoError,
/// <summary>
/// percent
/// </summary>
percent
YouTubeVideoError
}
}

View File

@ -2783,7 +2783,7 @@ Unimplemented Method: {2}</value>
<data name="PingPrefUrl" xml:space="preserve">
<value>&amp;Ping the following web addresses (one web address for each line)</value>
</data>
<data name="pixels" xml:space="preserve">
<data name="Pixels" xml:space="preserve">
<value>pixels</value>
</data>
<data name="Plugin_Map_InsertableContentSource_MenuText" xml:space="preserve">
@ -4486,7 +4486,7 @@ This might take a while.</value>
<data name="UnexpectedErrorSendError" xml:space="preserve">
<value>&amp;Send Error</value>
</data>
<data name="percent" xml:space="preserve">
<data name="Percent" xml:space="preserve">
<value>percent</value>
</data>
</root>

View File

@ -82,16 +82,16 @@ namespace OpenLiveWriter.PostEditor.PostHtmlEditing
numericLargeHeight.Enter += new EventHandler(numeric_Enter);
numericLargeWidth.Enter += new EventHandler(numeric_Enter);
this.label4.Text = Res.Get(StringId.pixels);
this.label3.Text = Res.Get(StringId.pixels);
this.label4.Text = Res.Get(StringId.Pixels);
this.label3.Text = Res.Get(StringId.Pixels);
this.label2.Text = Res.Get(StringId.ImgSBMaximumHeightLabel1);
this.label1.Text = Res.Get(StringId.ImgSBMaximumWidthLabel1);
this.label5.Text = Res.Get(StringId.pixels);
this.label6.Text = Res.Get(StringId.pixels);
this.label5.Text = Res.Get(StringId.Pixels);
this.label6.Text = Res.Get(StringId.Pixels);
this.label7.Text = Res.Get(StringId.ImgSBMaximumHeightLabel2);
this.label8.Text = Res.Get(StringId.ImgSBMaximumWidthLabel2);
this.label9.Text = Res.Get(StringId.pixels);
this.label10.Text = Res.Get(StringId.pixels);
this.label9.Text = Res.Get(StringId.Pixels);
this.label10.Text = Res.Get(StringId.Pixels);
this.label11.Text = Res.Get(StringId.ImgSBMaximumHeightLabel3);
this.label12.Text = Res.Get(StringId.ImgSBMaximumWidthLabel3);
this.buttonCancel.Text = Res.Get(StringId.CancelButton);

View File

@ -31,8 +31,8 @@ namespace OpenLiveWriter.PostEditor.Tables
// This call is required by the Windows.Forms Form Designer.
InitializeComponent();
this.labelWidth.Text = Res.Get(StringId.WidthLabel);
this.rbPixels.Text = Res.Get(StringId.pixels);
this.rbPercent.Text = Res.Get(StringId.percent);
this.rbPixels.Text = Res.Get(StringId.Pixels);
this.rbPercent.Text = Res.Get(StringId.Percent);
}
protected override void OnLoad(EventArgs e)

View File

@ -40,7 +40,7 @@ namespace OpenLiveWriter.PostEditor.Tables
{
get
{
return new PixelPercent((string) _baseCell.width); //TableHelper.GetCellWidth(_baseCell);
return new PixelPercent((string) _baseCell.width, CultureInfo.CurrentCulture);
}
set
{

View File

@ -31,7 +31,7 @@ namespace OpenLiveWriter.PostEditor.Tables
Value = value;
}
public PixelPercent(string text, IFormatProvider provider, PixelPercentUnits units)
public PixelPercent(string text, IFormatProvider provider, PixelPercentUnits units) : this()
{
if (string.IsNullOrEmpty(text))
{
@ -43,9 +43,16 @@ namespace OpenLiveWriter.PostEditor.Tables
var s = text.Trim();
Units = units;
var value = int.Parse(s, provider);
Value = value;
int value;
if (int.TryParse(s, NumberStyles.Integer, provider, out value))
{
Value = value;
}
else
{
Units = PixelPercentUnits.Undefined;
Value = 0;
}
}
}
@ -67,28 +74,41 @@ namespace OpenLiveWriter.PostEditor.Tables
s = s.TrimEnd('%');
}
var value = int.Parse(s, provider);
Value = value;
Units = units;
int value;
if (int.TryParse(s, NumberStyles.Integer, provider, out value))
{
Value = value;
Units = units;
}
else
{
Units = PixelPercentUnits.Undefined;
}
}
}
public static bool CanParse(string text)
{
try
if (string.IsNullOrEmpty(text))
{
var p = new PixelPercent(text, CultureInfo.InvariantCulture);
return true;
}
catch (Exception)
{
return false;
}
}
public PixelPercent(string text) : this(text, CultureInfo.CurrentCulture)
{
var s = text.Trim();
if (s.EndsWith("%"))
{
s = s.TrimEnd('%');
}
int value;
if (int.TryParse(s, NumberStyles.Integer, CultureInfo.InvariantCulture, out value))
{
return true;
}
return false;
}
public int Value { get; private set; }

View File

@ -40,7 +40,7 @@ namespace OpenLiveWriter.PostEditor.Tables
this.buttonOK.Text = Res.Get(StringId.OKButtonText);
this.buttonCancel.Text = Res.Get(StringId.CancelButton);
this.groupBoxHeight.Text = Res.Get(StringId.Height);
this.labelPixels.Text = Res.Get(StringId.pixels);
this.labelPixels.Text = Res.Get(StringId.Pixels);
this.radioButtonFixedHeight.Text = Res.Get(StringId.RowPropertiesFixedHeight);
this.radioButtonSizeToContent.Text = Res.Get(StringId.RowPropertiesSizeToContent);
this.Text = Res.Get(StringId.RowPropertiesTitle);

View File

@ -28,9 +28,9 @@ namespace OpenLiveWriter.PostEditor.Tables
this.labelRows.Text = Res.Get(StringId.TableRowsLabel);
this.labelColumns.Text = Res.Get(StringId.TableColumnsLabel);
this.groupBoxAppearance.Text = Res.Get(StringId.Appearance);
this.labelSpacingPixels.Text = Res.Get(StringId.pixels);
this.labelPaddingPixels.Text = Res.Get(StringId.pixels);
this.labelBorderPixels.Text = Res.Get(StringId.pixels);
this.labelSpacingPixels.Text = Res.Get(StringId.Pixels);
this.labelPaddingPixels.Text = Res.Get(StringId.Pixels);
this.labelBorderPixels.Text = Res.Get(StringId.Pixels);
this.checkBoxShowBorder.Text = Res.Get(StringId.TableShowBorderLabel);
this.label3.Text = Res.Get(StringId.TableCellSpacingLabel);
this.label1.Text = Res.Get(StringId.TableCellPaddingLabel);

View File

@ -87,6 +87,7 @@
</Otherwise>
</Choose>
<ItemGroup>
<Compile Include="PostEditor\Tables\PixelPercentTests.cs" />
<Compile Include="PostEditor\Tables\TestHtmlEditor.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
<Compile Include="PostEditor\Tables\InsertTableTests.cs" />

View File

@ -0,0 +1,116 @@
using System;
using System.Globalization;
using NUnit.Framework;
using OpenLiveWriter.PostEditor.Tables;
namespace OpenLiveWriter.Tests.PostEditor.Tables
{
[TestFixture]
public class PixelPercentTests
{
[Test]
public void Default_Constructor()
{
// Act
var sut = new PixelPercent();
// Assert
Assert.AreEqual(0, sut.Value);
Assert.AreEqual(PixelPercentUnits.Undefined, sut.Units);
}
[Test]
[TestCase(1, PixelPercentUnits.Percentage)]
[TestCase(100, PixelPercentUnits.Percentage)]
[TestCase(2, PixelPercentUnits.Pixels)]
[TestCase(300, PixelPercentUnits.Pixels)]
public void Constructor_Valid_Values(int value, PixelPercentUnits units)
{
// Act
var sut = new PixelPercent(value, units);
// Assert
Assert.AreEqual(value, sut.Value);
Assert.AreEqual(units, sut.Units);
}
[Test]
[TestCase(-1, PixelPercentUnits.Percentage)]
[TestCase(101, PixelPercentUnits.Percentage)]
[TestCase(-1, PixelPercentUnits.Pixels)]
public void Constructor_Invalid_Values(int value, PixelPercentUnits units)
{
// Assert
Assert.Throws<ArgumentOutOfRangeException>(() => new PixelPercent(value, units));
}
[Test]
[TestCase(null)]
[TestCase("")]
public void Constructor_EmptyValues_Gives_Undefined(string text)
{
var sut = new PixelPercent(text, CultureInfo.InvariantCulture);
// Assert
Assert.AreEqual(PixelPercentUnits.Undefined, sut.Units);
}
[Test]
[TestCase("1", 1, PixelPercentUnits.Pixels)]
[TestCase("1%", 1, PixelPercentUnits.Percentage)]
[TestCase(" 100% ", 100, PixelPercentUnits.Percentage)]
public void Constructor_Valid_Values(string text, int expectedValue, PixelPercentUnits expectedUnits)
{
// Act
var sut = new PixelPercent(text, CultureInfo.InvariantCulture);
// Assert
Assert.AreEqual(expectedUnits, sut.Units);
Assert.AreEqual(expectedValue, sut.Value);
}
[Test]
[TestCase("1", 1, PixelPercentUnits.Pixels)]
[TestCase("1", 1, PixelPercentUnits.Percentage)]
[TestCase(" 100 ", 100, PixelPercentUnits.Percentage)]
public void Constructor_Valid_Values_With_Units(string text, int expectedValue, PixelPercentUnits units)
{
// Act
var sut = new PixelPercent(text, CultureInfo.InvariantCulture, units);
// Assert
Assert.AreEqual(expectedValue, sut.Value);
}
[Test]
[TestCase("d", PixelPercentUnits.Pixels)]
[TestCase("1%", PixelPercentUnits.Percentage)]
[TestCase("100.454 ", PixelPercentUnits.Percentage)]
public void Constructor_Invalid_Values_With_Units(string text, PixelPercentUnits units)
{
// Act
var sut = new PixelPercent(text, CultureInfo.InvariantCulture, units);
// Assert
Assert.AreEqual(0, sut.Value);
Assert.AreEqual(PixelPercentUnits.Undefined, sut.Units);
}
[Test]
[TestCase("0", true)]
[TestCase("100", true)]
[TestCase("100%", true)]
[TestCase("x", false)]
[TestCase("107.9", false)]
public void CanParse_Values(string text, bool expected)
{
// Act
var result = PixelPercent.CanParse(text);
// Assert
Assert.AreEqual(expected, result);
}
}
}