Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Added new snippets. #196

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added new snippets. #196

wants to merge 1 commit into from

Conversation

Dennis-Petrov
Copy link

Snippets use string.IsNullOrWhiteSpace() instead of
string.IsNullOrEmpty, as proposed in #190.

Snippets use string.IsNullOrWhiteSpace() instead of
string.IsNullOrEmpty.
@sharwell
Copy link
Member

sharwell commented Aug 8, 2015

💡 If you add new snippets, you should add their shortcuts to shortcuts.txt (in the same folder). They will also need to be added to the CSharpSnippets in Microsoft.Research/ManagedContract.Setup/ManagedContracts.wxs. If you aren't comfortable with the last one, just open up a new issue and we can address it separately if this gets merged. 😄

📝 This pull request does not supersede (or conflict with) #193.

💭 I am leaning slightly towards #193 (which changes String to string but still uses IsNullOrEmpty). I find that most cases in code which use IsNullOrWhiteSpace incur a performance cost but gain very little in terms of argument validation. I'll wait for feedback from others though.

<Title>Contract.Ensures(!string.IsNullOrWhiteSpace(Contract.Result()))</Title>
<Shortcut>cesnw</Shortcut>
<Description>Emits an 'ensures' clause specifying result not null or white space</Description>
<Author>Jonathan de Halleux</Author>
Copy link
Member

Choose a reason for hiding this comment

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

💡 You could change this to your name instead. 😄

@Dennis-Petrov
Copy link
Author

Thanks, I'll fix shorcuts.txt/WiX script a little later. Concerning performance, these snippets, of course, are not replacement for IsNullOrEmpty family, and should be used only when white space strings are not allowed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants