Skip to content

Update DisplayInterface.cs - use StringBuilder instead of concatenation to print#29

Merged
rpavlik merged 3 commits intoOSVR:masterfrom
stellatigre:StellaTigre-stringbuilder
Mar 17, 2015
Merged

Update DisplayInterface.cs - use StringBuilder instead of concatenation to print#29
rpavlik merged 3 commits intoOSVR:masterfrom
stellatigre:StellaTigre-stringbuilder

Conversation

@stellatigre
Copy link
Copy Markdown
Contributor

This commit changes the long series of string concatenations to print the parsed results in this class with a StringBuilder. This code runs faster (granted, by a millisecond or two), as that's what StringBuilder is for. I think it is also cleaner than the previous code - manually adding newlines like that seemed messy.

In addition, the original code used width where it really seems height should have been used - that has also been changed. Hopefully this is correct ?

@rpavlik
Copy link
Copy Markdown
Member

rpavlik commented Mar 16, 2015

At least the concept of the string changes sounds great. @DuFF14 can you review this? Will it merge with your changes that are also in a pull request?

@DuFF14
Copy link
Copy Markdown
Member

DuFF14 commented Mar 17, 2015

Yes, good changes and good catch on the "width" typo. Shouldn't interfere with my pull request.

On Mar 16, 2015, at 6:31 PM, Ryan Pavlik notifications@github.com wrote:

At least the concept of the string changes sounds great. @DuFF14 can you review this? Will it merge with your changes that are also in a pull request?


Reply to this email directly or view it on GitHub.

rpavlik added a commit that referenced this pull request Mar 17, 2015
Update DisplayInterface.cs - use StringBuilder instead of concatenation to print
@rpavlik rpavlik merged commit a8bf8c4 into OSVR:master Mar 17, 2015
@rpavlik
Copy link
Copy Markdown
Member

rpavlik commented Mar 17, 2015

Thanks for your contribution, @StellaCannefax !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants