Skip to content

Spin() function added #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nawinkhatiwada wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Spin() function added #2

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
03edf3d
Spin() function added
ersuman Jun 13, 2017
63cc674
Update RotaryWheel.xaml.cs
ersuman Aug 29, 2017
aea5a98
Update RotaryWheel.xaml.cs
ersuman Aug 29, 2017
50c8b07
Update RotaryWheel.xaml.cs
ersuman Aug 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 50 additions & 2 deletions RotaryWheelControl/RotaryWheel.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ public partial class RotaryWheel : UserControl, INotifyPropertyChanged
public Color BackgroundColor
{
get { return _backgroundColor; }
set { SetField(ref _backgroundColor, value); }
set { SetField(ref _backgroundColor, value);
Draw();

@jpoon jpoon Jul 6, 2017 •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to add the Draw()? Setting a new color should automatically be handled as this class inherits from INotifyPropertyChanged...

@ersuman ersuman Jul 7, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, i've experienced that without Draw() function call, BackgroundColor doesnot changes instantly

}
}

private Color _foregroundColor = Colors.White;
Expand Down Expand Up @@ -143,6 +145,52 @@ public RotaryWheel()
};
}

private void SpinTo(int itemIndex, int durationInSec = -1)
{
Random r = new Random(DateTime.Now.Millisecond);
var angleFromYAxis = 360 - Angle;
SelectedItem = _pieSlices

@jpoon jpoon Jul 6, 2017

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you are getting the currently selected pie slice? Why not use the SelectedItem getter?

.SingleOrDefault(p => p.StartAngle <= angleFromYAxis && (p.StartAngle + p.Angle) > angleFromYAxis);

int count = _pieSlices.Count;
int currIndex = _pieSlices.IndexOf(SelectedItem);
int fullSpin = itemIndex / count;
itemIndex = itemIndex % count;
int steps = currIndex-itemIndex;

@jpoon jpoon Jul 6, 2017

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider not modifying the itemIndex value as it would then no longer be "item index". Instead,

int steps = currIndex - itemIndex % count;

if (steps < 0)

@jpoon jpoon Jul 6, 2017

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, can you add { and }?

steps = count + steps;

var startAngle = SelectedItem.StartAngle + SelectedItem.Angle / 2;

@jpoon jpoon Jul 6, 2017 •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't SelectedItem.StartAngle be sufficient? Why is the .Angle/2 necessary? I'm a couple years removed from this codebase so not sure if I'm missing something.

@ersuman ersuman Jul 7, 2017 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to rotate the pieslice at middle :)

var finalAngle = startAngle + fullSpin*360 + steps*360/count;

doubleAnimation.From = startAngle;
doubleAnimation.To = finalAngle;
if(durationInSec>0)

@jpoon jpoon Jul 6, 2017 •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, can you add { and } around the if block.

doubleAnimation.Duration = new Windows.UI.Xaml.Duration(new TimeSpan(0, 0, durationInSec));

@jpoon jpoon Jul 6, 2017

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use TimeSpan.FromSeconds() (https://msdn.microsoft.com/en-us/library/system.timespan.fromseconds(v=vs.110).aspx) instead?

else
doubleAnimation.Duration = new Windows.UI.Xaml.Duration(new TimeSpan(0, 0, r.Next(3, 6)));
storyBoard.Begin();
storyBoard.Completed += StoryBoard_Completed;

@jpoon jpoon Jul 6, 2017

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please set the .Completed event handler before you start the Begin() to prevent any race conditions.

@jpoon jpoon Jul 6, 2017

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for StoryBoard_Completed is fairly simple so I'd put that into an anonymous function.

Angle = ((int)finalAngle) % 360;

}
/// <summary>
/// Spins the wheel randomly.
/// </summary>
/// <param name="maxSpins">Maximum no. of spins or revolutions.</param>
/// <param name="durationInSec">Spin duration in Second. [-1 denotes random duration]</param>
public void Spin(int maxSpins=5, int durationInSec = -1)

@jpoon jpoon Jul 6, 2017

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the Spin function is public, can you move it underneath the constructor (above the private functions).

{
Random r = new Random(DateTime.Now.Millisecond);

@jpoon jpoon Jul 6, 2017

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's necessary to pass in a seed value, the default value should be sufficient.

@ersuman ersuman Jul 7, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if someone wants to spin more :)

@jpoon jpoon Jul 10, 2017

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seed defines how the random numbers will be generated. As you are using DateTime.Now.Milliseconds. It's equivalent to using the default value (ie. passing in no argument).

@ersuman ersuman Aug 29, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! yes, you're right.

int steps = r.Next(_pieSlices.Count, _pieSlices.Count* maxSpins);
SpinTo(steps,durationInSec);
}
private void StoryBoard_Completed(object sender, object e)
{
var angleFromYAxis = 360 - Angle;
SelectedItem = _pieSlices
.SingleOrDefault(p => p.StartAngle <= angleFromYAxis && (p.StartAngle + p.Angle) > angleFromYAxis);
}
private void Draw()
{
_pieSlices.Clear();
Expand Down Expand Up @@ -210,4 +258,4 @@ private void SetField<T>(ref T field, T value, [CallerMemberName] string propert
}
}
}
}
}