Replace all the occurrence of a string

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
8
down vote

favorite












I have the following function to replace all the occurrence of a string that matches certain token.



public string ReplaceTokenBySample(string StingValue)

List<Token> tokens = GetTokenList();
foreach (var token in tokens)

StingValue = StingValue.Replace(token.Token, token.SampleValue);

return StingValue;



GetTokenList(); will return



Token SampleValue 
##Username## John Doe
##UserEmail## john.doe@domain.com
##UserFirstName## John
##UserLastName## Doe


How can I optimize this code?



Full console app code is as follows:



using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;

namespace Rextester

public class Token

public string TokenValue get; set;
public string SampleValue get; set;

public class Program

public List<Token>GetTokenList()

List<Token> tokens = new List<Token>

new Token() TokenValue = "##Username##", SampleValue="John Doe" ,
new Token() TokenValue = "##UserEmail##", SampleValue="john.doe@domain.com " ,
new Token() TokenValue = "##UserFirstName##", SampleValue="John" ,
new Token() TokenValue = "##UserLastName##", SampleValue="Doe"
;
return tokens;


public string ReplaceTokenBySample(string StringValue)

List<Token> tokens = GetTokenList();
foreach (var token in tokens)

StringValue = StringValue.Replace(token.TokenValue, token.SampleValue);

return StringValue;


public static void Main(string args)

Program obj = new Program();
string StringValue ="Hello ##Username##! I have emailed you at ##UserEmail##. ##UserFirstName## ##UserLastName## how is you days going on. Have a good day ##Username##. ";
Console.WriteLine(obj.ReplaceTokenBySample(StringValue));









share|improve this question





















  • What's the point of this token system if ReplaceTokenBySample doesn't allow you to pass in your own tokens?
    – Pieter Witvoet
    Jul 12 at 12:47










  • @PieterWitvoet token and sample value will be stored on database. And here StingValue will be replace by Mail Template. User are allowed to create any number of token..
    – Kiran Shahi
    Jul 12 at 14:58






  • 1




    That's not what the code you have shown is doing, however. If you're planning to modify GetTokenList, then ReplaceTokenBySample is limited to always use whatever tokens are returned by GetTokenList. If, instead, you pass tokens in as an argument, then you can reuse the replacement logic in different contexts.
    – Pieter Witvoet
    Jul 13 at 8:18
















up vote
8
down vote

favorite












I have the following function to replace all the occurrence of a string that matches certain token.



public string ReplaceTokenBySample(string StingValue)

List<Token> tokens = GetTokenList();
foreach (var token in tokens)

StingValue = StingValue.Replace(token.Token, token.SampleValue);

return StingValue;



GetTokenList(); will return



Token SampleValue 
##Username## John Doe
##UserEmail## john.doe@domain.com
##UserFirstName## John
##UserLastName## Doe


How can I optimize this code?



Full console app code is as follows:



using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;

namespace Rextester

public class Token

public string TokenValue get; set;
public string SampleValue get; set;

public class Program

public List<Token>GetTokenList()

List<Token> tokens = new List<Token>

new Token() TokenValue = "##Username##", SampleValue="John Doe" ,
new Token() TokenValue = "##UserEmail##", SampleValue="john.doe@domain.com " ,
new Token() TokenValue = "##UserFirstName##", SampleValue="John" ,
new Token() TokenValue = "##UserLastName##", SampleValue="Doe"
;
return tokens;


public string ReplaceTokenBySample(string StringValue)

List<Token> tokens = GetTokenList();
foreach (var token in tokens)

StringValue = StringValue.Replace(token.TokenValue, token.SampleValue);

return StringValue;


public static void Main(string args)

Program obj = new Program();
string StringValue ="Hello ##Username##! I have emailed you at ##UserEmail##. ##UserFirstName## ##UserLastName## how is you days going on. Have a good day ##Username##. ";
Console.WriteLine(obj.ReplaceTokenBySample(StringValue));









share|improve this question





















  • What's the point of this token system if ReplaceTokenBySample doesn't allow you to pass in your own tokens?
    – Pieter Witvoet
    Jul 12 at 12:47










  • @PieterWitvoet token and sample value will be stored on database. And here StingValue will be replace by Mail Template. User are allowed to create any number of token..
    – Kiran Shahi
    Jul 12 at 14:58






  • 1




    That's not what the code you have shown is doing, however. If you're planning to modify GetTokenList, then ReplaceTokenBySample is limited to always use whatever tokens are returned by GetTokenList. If, instead, you pass tokens in as an argument, then you can reuse the replacement logic in different contexts.
    – Pieter Witvoet
    Jul 13 at 8:18












up vote
8
down vote

favorite









up vote
8
down vote

favorite











I have the following function to replace all the occurrence of a string that matches certain token.



public string ReplaceTokenBySample(string StingValue)

List<Token> tokens = GetTokenList();
foreach (var token in tokens)

StingValue = StingValue.Replace(token.Token, token.SampleValue);

return StingValue;



GetTokenList(); will return



Token SampleValue 
##Username## John Doe
##UserEmail## john.doe@domain.com
##UserFirstName## John
##UserLastName## Doe


How can I optimize this code?



Full console app code is as follows:



using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;

namespace Rextester

public class Token

public string TokenValue get; set;
public string SampleValue get; set;

public class Program

public List<Token>GetTokenList()

List<Token> tokens = new List<Token>

new Token() TokenValue = "##Username##", SampleValue="John Doe" ,
new Token() TokenValue = "##UserEmail##", SampleValue="john.doe@domain.com " ,
new Token() TokenValue = "##UserFirstName##", SampleValue="John" ,
new Token() TokenValue = "##UserLastName##", SampleValue="Doe"
;
return tokens;


public string ReplaceTokenBySample(string StringValue)

List<Token> tokens = GetTokenList();
foreach (var token in tokens)

StringValue = StringValue.Replace(token.TokenValue, token.SampleValue);

return StringValue;


public static void Main(string args)

Program obj = new Program();
string StringValue ="Hello ##Username##! I have emailed you at ##UserEmail##. ##UserFirstName## ##UserLastName## how is you days going on. Have a good day ##Username##. ";
Console.WriteLine(obj.ReplaceTokenBySample(StringValue));









share|improve this question













I have the following function to replace all the occurrence of a string that matches certain token.



public string ReplaceTokenBySample(string StingValue)

List<Token> tokens = GetTokenList();
foreach (var token in tokens)

StingValue = StingValue.Replace(token.Token, token.SampleValue);

return StingValue;



GetTokenList(); will return



Token SampleValue 
##Username## John Doe
##UserEmail## john.doe@domain.com
##UserFirstName## John
##UserLastName## Doe


How can I optimize this code?



Full console app code is as follows:



using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;

namespace Rextester

public class Token

public string TokenValue get; set;
public string SampleValue get; set;

public class Program

public List<Token>GetTokenList()

List<Token> tokens = new List<Token>

new Token() TokenValue = "##Username##", SampleValue="John Doe" ,
new Token() TokenValue = "##UserEmail##", SampleValue="john.doe@domain.com " ,
new Token() TokenValue = "##UserFirstName##", SampleValue="John" ,
new Token() TokenValue = "##UserLastName##", SampleValue="Doe"
;
return tokens;


public string ReplaceTokenBySample(string StringValue)

List<Token> tokens = GetTokenList();
foreach (var token in tokens)

StringValue = StringValue.Replace(token.TokenValue, token.SampleValue);

return StringValue;


public static void Main(string args)

Program obj = new Program();
string StringValue ="Hello ##Username##! I have emailed you at ##UserEmail##. ##UserFirstName## ##UserLastName## how is you days going on. Have a good day ##Username##. ";
Console.WriteLine(obj.ReplaceTokenBySample(StringValue));











share|improve this question












share|improve this question




share|improve this question








edited Jul 12 at 11:05









t3chb0t

31.8k54095




31.8k54095









asked Jul 12 at 10:06









Kiran Shahi

1536




1536











  • What's the point of this token system if ReplaceTokenBySample doesn't allow you to pass in your own tokens?
    – Pieter Witvoet
    Jul 12 at 12:47










  • @PieterWitvoet token and sample value will be stored on database. And here StingValue will be replace by Mail Template. User are allowed to create any number of token..
    – Kiran Shahi
    Jul 12 at 14:58






  • 1




    That's not what the code you have shown is doing, however. If you're planning to modify GetTokenList, then ReplaceTokenBySample is limited to always use whatever tokens are returned by GetTokenList. If, instead, you pass tokens in as an argument, then you can reuse the replacement logic in different contexts.
    – Pieter Witvoet
    Jul 13 at 8:18
















  • What's the point of this token system if ReplaceTokenBySample doesn't allow you to pass in your own tokens?
    – Pieter Witvoet
    Jul 12 at 12:47










  • @PieterWitvoet token and sample value will be stored on database. And here StingValue will be replace by Mail Template. User are allowed to create any number of token..
    – Kiran Shahi
    Jul 12 at 14:58






  • 1




    That's not what the code you have shown is doing, however. If you're planning to modify GetTokenList, then ReplaceTokenBySample is limited to always use whatever tokens are returned by GetTokenList. If, instead, you pass tokens in as an argument, then you can reuse the replacement logic in different contexts.
    – Pieter Witvoet
    Jul 13 at 8:18















What's the point of this token system if ReplaceTokenBySample doesn't allow you to pass in your own tokens?
– Pieter Witvoet
Jul 12 at 12:47




What's the point of this token system if ReplaceTokenBySample doesn't allow you to pass in your own tokens?
– Pieter Witvoet
Jul 12 at 12:47












@PieterWitvoet token and sample value will be stored on database. And here StingValue will be replace by Mail Template. User are allowed to create any number of token..
– Kiran Shahi
Jul 12 at 14:58




@PieterWitvoet token and sample value will be stored on database. And here StingValue will be replace by Mail Template. User are allowed to create any number of token..
– Kiran Shahi
Jul 12 at 14:58




1




1




That's not what the code you have shown is doing, however. If you're planning to modify GetTokenList, then ReplaceTokenBySample is limited to always use whatever tokens are returned by GetTokenList. If, instead, you pass tokens in as an argument, then you can reuse the replacement logic in different contexts.
– Pieter Witvoet
Jul 13 at 8:18




That's not what the code you have shown is doing, however. If you're planning to modify GetTokenList, then ReplaceTokenBySample is limited to always use whatever tokens are returned by GetTokenList. If, instead, you pass tokens in as an argument, then you can reuse the replacement logic in different contexts.
– Pieter Witvoet
Jul 13 at 8:18










2 Answers
2






active

oldest

votes

















up vote
8
down vote



accepted










Loops can sometimes be so ugly. You can get rid of them completely and use Regex instead. It'll replace the placeholders one by one without scanning the string from the beginning each time. Just put your placeholder and the corresponding values in a dictionary and let Regex.Replace do the job.



var replacements = new Dictionary<string, string>

["Username"] = "John Doe",
["UserEmail"] = "john.doe@domain.com",
["UserFirstName"] = "John",
["UserLastName"] = "Doe"
;

var value = "Hello ##Username##! I have emailed you at ##UserEmail##. ##UserFirstName## ##UserLastName## how is you days going on. Have a good day ##Username##. ";

var pattern = $"##(?<placeholder>", replacements.Keys))##";
var result = Regex.Replace(value, pattern, m => replacements[m.Groups["placeholder"].Value], RegexOptions.ExplicitCapture);





share|improve this answer

















  • 1




    I always forget regex...this should perform pretty well and it's also really readable!!!
    – Adriano Repetti
    Jul 13 at 6:59










  • Hi @t3chb0t would it be costly to convert list to dictionary? Because list is return from database.
    – Kiran Shahi
    Jul 13 at 8:38










  • @KiranShahi there is no general answer to this question. It depends on many factors: how long is the list? how long are the strings? how many of them you are processing per second/minute/hour? It doesn't have to be a dictionary. You can look the values up in the list with .Single but agian... it depends on many many different factors.
    – t3chb0t
    Jul 13 at 8:42











  • @t3chb0t is there any way to use the answer you have mention with list? Actually I don't want loop as well as type conversion
    – Kiran Shahi
    Jul 13 at 8:44







  • 1




    @KiranShahi exactly... when it's working then you've implemented it correctly ;-)
    – t3chb0t
    Jul 13 at 9:05

















up vote
13
down vote













If this is your real use-case then I'd not go beyond the obvious optimizations. In this case you're creating a new string for each replacement, you can use StringBuilder.Replace() instead:



public string ReplaceTokenBySample(string value)

var result = new StringBuilder(value);
GetTokenList().ForEach(x => result.Replace(x.TokenValue, x.SampleValue));

return result.ToString();



If instead of List<Token> you have IEnumerable<Token> (or if you want to avoid List<T>.ForEach()) then you can use:



public string ReplaceTokenBySample(string value)

GetTokenList().Aggregate(new StringBuilder(value),
(result, item) => result.Replace(item.TokenValue, item.SampleValue));

return result.ToString();



Few things to consider:



  • You're performing ordinal case-sensitive comparison, it seems to be appropriate to replace placeholders with their actual value and it has the benefit to be reasonably fast. If this is not the case then you should definitely go with t3schb0t's answer (which handles this properly and it's incredibly more readable.)

  • Parameters should be camelCase.

  • It's usually confusing to overwrite parameters value with something else (especially for non trivial code), compiler is able to optimize your code to get the best of it even when using a local variable.

Few more notes about overall design: you do not validate your parameters anywhere, if they're only used internally to your assembly then mark them internal or private (as appropriate) and ASSERT about their content (for example: why do you declare Token as public?)



Names sound little bit strange, you do not need to repeat the type in the name: TokenSample and TokenValue inside the Token class (and StringValue for a value of type String).



If you intend to fill Token programmatically then adding a Token(string, string) ctor may help to keep your code shorter.






share|improve this answer























  • Thank you for you answer @AdrianoRepetti I have find answer by t3chb0t more suitable :) Thank you for your time.
    – Kiran Shahi
    Jul 13 at 9:03











  • @KiranShahi you welcome; unless you strictly need ordinal comparison then t3's answer should perform really well and it's much more readable.
    – Adriano Repetti
    Jul 13 at 14:55










Your Answer




StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");

StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");

StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);

else
createEditor();

);

function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: false,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);



);








 

draft saved


draft discarded


















StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f198350%2freplace-all-the-occurrence-of-a-string%23new-answer', 'question_page');

);

Post as a guest






























2 Answers
2






active

oldest

votes








2 Answers
2






active

oldest

votes









active

oldest

votes






active

oldest

votes








up vote
8
down vote



accepted










Loops can sometimes be so ugly. You can get rid of them completely and use Regex instead. It'll replace the placeholders one by one without scanning the string from the beginning each time. Just put your placeholder and the corresponding values in a dictionary and let Regex.Replace do the job.



var replacements = new Dictionary<string, string>

["Username"] = "John Doe",
["UserEmail"] = "john.doe@domain.com",
["UserFirstName"] = "John",
["UserLastName"] = "Doe"
;

var value = "Hello ##Username##! I have emailed you at ##UserEmail##. ##UserFirstName## ##UserLastName## how is you days going on. Have a good day ##Username##. ";

var pattern = $"##(?<placeholder>", replacements.Keys))##";
var result = Regex.Replace(value, pattern, m => replacements[m.Groups["placeholder"].Value], RegexOptions.ExplicitCapture);





share|improve this answer

















  • 1




    I always forget regex...this should perform pretty well and it's also really readable!!!
    – Adriano Repetti
    Jul 13 at 6:59










  • Hi @t3chb0t would it be costly to convert list to dictionary? Because list is return from database.
    – Kiran Shahi
    Jul 13 at 8:38










  • @KiranShahi there is no general answer to this question. It depends on many factors: how long is the list? how long are the strings? how many of them you are processing per second/minute/hour? It doesn't have to be a dictionary. You can look the values up in the list with .Single but agian... it depends on many many different factors.
    – t3chb0t
    Jul 13 at 8:42











  • @t3chb0t is there any way to use the answer you have mention with list? Actually I don't want loop as well as type conversion
    – Kiran Shahi
    Jul 13 at 8:44







  • 1




    @KiranShahi exactly... when it's working then you've implemented it correctly ;-)
    – t3chb0t
    Jul 13 at 9:05














up vote
8
down vote



accepted










Loops can sometimes be so ugly. You can get rid of them completely and use Regex instead. It'll replace the placeholders one by one without scanning the string from the beginning each time. Just put your placeholder and the corresponding values in a dictionary and let Regex.Replace do the job.



var replacements = new Dictionary<string, string>

["Username"] = "John Doe",
["UserEmail"] = "john.doe@domain.com",
["UserFirstName"] = "John",
["UserLastName"] = "Doe"
;

var value = "Hello ##Username##! I have emailed you at ##UserEmail##. ##UserFirstName## ##UserLastName## how is you days going on. Have a good day ##Username##. ";

var pattern = $"##(?<placeholder>", replacements.Keys))##";
var result = Regex.Replace(value, pattern, m => replacements[m.Groups["placeholder"].Value], RegexOptions.ExplicitCapture);





share|improve this answer

















  • 1




    I always forget regex...this should perform pretty well and it's also really readable!!!
    – Adriano Repetti
    Jul 13 at 6:59










  • Hi @t3chb0t would it be costly to convert list to dictionary? Because list is return from database.
    – Kiran Shahi
    Jul 13 at 8:38










  • @KiranShahi there is no general answer to this question. It depends on many factors: how long is the list? how long are the strings? how many of them you are processing per second/minute/hour? It doesn't have to be a dictionary. You can look the values up in the list with .Single but agian... it depends on many many different factors.
    – t3chb0t
    Jul 13 at 8:42











  • @t3chb0t is there any way to use the answer you have mention with list? Actually I don't want loop as well as type conversion
    – Kiran Shahi
    Jul 13 at 8:44







  • 1




    @KiranShahi exactly... when it's working then you've implemented it correctly ;-)
    – t3chb0t
    Jul 13 at 9:05












up vote
8
down vote



accepted







up vote
8
down vote



accepted






Loops can sometimes be so ugly. You can get rid of them completely and use Regex instead. It'll replace the placeholders one by one without scanning the string from the beginning each time. Just put your placeholder and the corresponding values in a dictionary and let Regex.Replace do the job.



var replacements = new Dictionary<string, string>

["Username"] = "John Doe",
["UserEmail"] = "john.doe@domain.com",
["UserFirstName"] = "John",
["UserLastName"] = "Doe"
;

var value = "Hello ##Username##! I have emailed you at ##UserEmail##. ##UserFirstName## ##UserLastName## how is you days going on. Have a good day ##Username##. ";

var pattern = $"##(?<placeholder>", replacements.Keys))##";
var result = Regex.Replace(value, pattern, m => replacements[m.Groups["placeholder"].Value], RegexOptions.ExplicitCapture);





share|improve this answer













Loops can sometimes be so ugly. You can get rid of them completely and use Regex instead. It'll replace the placeholders one by one without scanning the string from the beginning each time. Just put your placeholder and the corresponding values in a dictionary and let Regex.Replace do the job.



var replacements = new Dictionary<string, string>

["Username"] = "John Doe",
["UserEmail"] = "john.doe@domain.com",
["UserFirstName"] = "John",
["UserLastName"] = "Doe"
;

var value = "Hello ##Username##! I have emailed you at ##UserEmail##. ##UserFirstName## ##UserLastName## how is you days going on. Have a good day ##Username##. ";

var pattern = $"##(?<placeholder>", replacements.Keys))##";
var result = Regex.Replace(value, pattern, m => replacements[m.Groups["placeholder"].Value], RegexOptions.ExplicitCapture);






share|improve this answer













share|improve this answer



share|improve this answer











answered Jul 12 at 17:22









t3chb0t

31.8k54095




31.8k54095







  • 1




    I always forget regex...this should perform pretty well and it's also really readable!!!
    – Adriano Repetti
    Jul 13 at 6:59










  • Hi @t3chb0t would it be costly to convert list to dictionary? Because list is return from database.
    – Kiran Shahi
    Jul 13 at 8:38










  • @KiranShahi there is no general answer to this question. It depends on many factors: how long is the list? how long are the strings? how many of them you are processing per second/minute/hour? It doesn't have to be a dictionary. You can look the values up in the list with .Single but agian... it depends on many many different factors.
    – t3chb0t
    Jul 13 at 8:42











  • @t3chb0t is there any way to use the answer you have mention with list? Actually I don't want loop as well as type conversion
    – Kiran Shahi
    Jul 13 at 8:44







  • 1




    @KiranShahi exactly... when it's working then you've implemented it correctly ;-)
    – t3chb0t
    Jul 13 at 9:05












  • 1




    I always forget regex...this should perform pretty well and it's also really readable!!!
    – Adriano Repetti
    Jul 13 at 6:59










  • Hi @t3chb0t would it be costly to convert list to dictionary? Because list is return from database.
    – Kiran Shahi
    Jul 13 at 8:38










  • @KiranShahi there is no general answer to this question. It depends on many factors: how long is the list? how long are the strings? how many of them you are processing per second/minute/hour? It doesn't have to be a dictionary. You can look the values up in the list with .Single but agian... it depends on many many different factors.
    – t3chb0t
    Jul 13 at 8:42











  • @t3chb0t is there any way to use the answer you have mention with list? Actually I don't want loop as well as type conversion
    – Kiran Shahi
    Jul 13 at 8:44







  • 1




    @KiranShahi exactly... when it's working then you've implemented it correctly ;-)
    – t3chb0t
    Jul 13 at 9:05







1




1




I always forget regex...this should perform pretty well and it's also really readable!!!
– Adriano Repetti
Jul 13 at 6:59




I always forget regex...this should perform pretty well and it's also really readable!!!
– Adriano Repetti
Jul 13 at 6:59












Hi @t3chb0t would it be costly to convert list to dictionary? Because list is return from database.
– Kiran Shahi
Jul 13 at 8:38




Hi @t3chb0t would it be costly to convert list to dictionary? Because list is return from database.
– Kiran Shahi
Jul 13 at 8:38












@KiranShahi there is no general answer to this question. It depends on many factors: how long is the list? how long are the strings? how many of them you are processing per second/minute/hour? It doesn't have to be a dictionary. You can look the values up in the list with .Single but agian... it depends on many many different factors.
– t3chb0t
Jul 13 at 8:42





@KiranShahi there is no general answer to this question. It depends on many factors: how long is the list? how long are the strings? how many of them you are processing per second/minute/hour? It doesn't have to be a dictionary. You can look the values up in the list with .Single but agian... it depends on many many different factors.
– t3chb0t
Jul 13 at 8:42













@t3chb0t is there any way to use the answer you have mention with list? Actually I don't want loop as well as type conversion
– Kiran Shahi
Jul 13 at 8:44





@t3chb0t is there any way to use the answer you have mention with list? Actually I don't want loop as well as type conversion
– Kiran Shahi
Jul 13 at 8:44





1




1




@KiranShahi exactly... when it's working then you've implemented it correctly ;-)
– t3chb0t
Jul 13 at 9:05




@KiranShahi exactly... when it's working then you've implemented it correctly ;-)
– t3chb0t
Jul 13 at 9:05












up vote
13
down vote













If this is your real use-case then I'd not go beyond the obvious optimizations. In this case you're creating a new string for each replacement, you can use StringBuilder.Replace() instead:



public string ReplaceTokenBySample(string value)

var result = new StringBuilder(value);
GetTokenList().ForEach(x => result.Replace(x.TokenValue, x.SampleValue));

return result.ToString();



If instead of List<Token> you have IEnumerable<Token> (or if you want to avoid List<T>.ForEach()) then you can use:



public string ReplaceTokenBySample(string value)

GetTokenList().Aggregate(new StringBuilder(value),
(result, item) => result.Replace(item.TokenValue, item.SampleValue));

return result.ToString();



Few things to consider:



  • You're performing ordinal case-sensitive comparison, it seems to be appropriate to replace placeholders with their actual value and it has the benefit to be reasonably fast. If this is not the case then you should definitely go with t3schb0t's answer (which handles this properly and it's incredibly more readable.)

  • Parameters should be camelCase.

  • It's usually confusing to overwrite parameters value with something else (especially for non trivial code), compiler is able to optimize your code to get the best of it even when using a local variable.

Few more notes about overall design: you do not validate your parameters anywhere, if they're only used internally to your assembly then mark them internal or private (as appropriate) and ASSERT about their content (for example: why do you declare Token as public?)



Names sound little bit strange, you do not need to repeat the type in the name: TokenSample and TokenValue inside the Token class (and StringValue for a value of type String).



If you intend to fill Token programmatically then adding a Token(string, string) ctor may help to keep your code shorter.






share|improve this answer























  • Thank you for you answer @AdrianoRepetti I have find answer by t3chb0t more suitable :) Thank you for your time.
    – Kiran Shahi
    Jul 13 at 9:03











  • @KiranShahi you welcome; unless you strictly need ordinal comparison then t3's answer should perform really well and it's much more readable.
    – Adriano Repetti
    Jul 13 at 14:55














up vote
13
down vote













If this is your real use-case then I'd not go beyond the obvious optimizations. In this case you're creating a new string for each replacement, you can use StringBuilder.Replace() instead:



public string ReplaceTokenBySample(string value)

var result = new StringBuilder(value);
GetTokenList().ForEach(x => result.Replace(x.TokenValue, x.SampleValue));

return result.ToString();



If instead of List<Token> you have IEnumerable<Token> (or if you want to avoid List<T>.ForEach()) then you can use:



public string ReplaceTokenBySample(string value)

GetTokenList().Aggregate(new StringBuilder(value),
(result, item) => result.Replace(item.TokenValue, item.SampleValue));

return result.ToString();



Few things to consider:



  • You're performing ordinal case-sensitive comparison, it seems to be appropriate to replace placeholders with their actual value and it has the benefit to be reasonably fast. If this is not the case then you should definitely go with t3schb0t's answer (which handles this properly and it's incredibly more readable.)

  • Parameters should be camelCase.

  • It's usually confusing to overwrite parameters value with something else (especially for non trivial code), compiler is able to optimize your code to get the best of it even when using a local variable.

Few more notes about overall design: you do not validate your parameters anywhere, if they're only used internally to your assembly then mark them internal or private (as appropriate) and ASSERT about their content (for example: why do you declare Token as public?)



Names sound little bit strange, you do not need to repeat the type in the name: TokenSample and TokenValue inside the Token class (and StringValue for a value of type String).



If you intend to fill Token programmatically then adding a Token(string, string) ctor may help to keep your code shorter.






share|improve this answer























  • Thank you for you answer @AdrianoRepetti I have find answer by t3chb0t more suitable :) Thank you for your time.
    – Kiran Shahi
    Jul 13 at 9:03











  • @KiranShahi you welcome; unless you strictly need ordinal comparison then t3's answer should perform really well and it's much more readable.
    – Adriano Repetti
    Jul 13 at 14:55












up vote
13
down vote










up vote
13
down vote









If this is your real use-case then I'd not go beyond the obvious optimizations. In this case you're creating a new string for each replacement, you can use StringBuilder.Replace() instead:



public string ReplaceTokenBySample(string value)

var result = new StringBuilder(value);
GetTokenList().ForEach(x => result.Replace(x.TokenValue, x.SampleValue));

return result.ToString();



If instead of List<Token> you have IEnumerable<Token> (or if you want to avoid List<T>.ForEach()) then you can use:



public string ReplaceTokenBySample(string value)

GetTokenList().Aggregate(new StringBuilder(value),
(result, item) => result.Replace(item.TokenValue, item.SampleValue));

return result.ToString();



Few things to consider:



  • You're performing ordinal case-sensitive comparison, it seems to be appropriate to replace placeholders with their actual value and it has the benefit to be reasonably fast. If this is not the case then you should definitely go with t3schb0t's answer (which handles this properly and it's incredibly more readable.)

  • Parameters should be camelCase.

  • It's usually confusing to overwrite parameters value with something else (especially for non trivial code), compiler is able to optimize your code to get the best of it even when using a local variable.

Few more notes about overall design: you do not validate your parameters anywhere, if they're only used internally to your assembly then mark them internal or private (as appropriate) and ASSERT about their content (for example: why do you declare Token as public?)



Names sound little bit strange, you do not need to repeat the type in the name: TokenSample and TokenValue inside the Token class (and StringValue for a value of type String).



If you intend to fill Token programmatically then adding a Token(string, string) ctor may help to keep your code shorter.






share|improve this answer















If this is your real use-case then I'd not go beyond the obvious optimizations. In this case you're creating a new string for each replacement, you can use StringBuilder.Replace() instead:



public string ReplaceTokenBySample(string value)

var result = new StringBuilder(value);
GetTokenList().ForEach(x => result.Replace(x.TokenValue, x.SampleValue));

return result.ToString();



If instead of List<Token> you have IEnumerable<Token> (or if you want to avoid List<T>.ForEach()) then you can use:



public string ReplaceTokenBySample(string value)

GetTokenList().Aggregate(new StringBuilder(value),
(result, item) => result.Replace(item.TokenValue, item.SampleValue));

return result.ToString();



Few things to consider:



  • You're performing ordinal case-sensitive comparison, it seems to be appropriate to replace placeholders with their actual value and it has the benefit to be reasonably fast. If this is not the case then you should definitely go with t3schb0t's answer (which handles this properly and it's incredibly more readable.)

  • Parameters should be camelCase.

  • It's usually confusing to overwrite parameters value with something else (especially for non trivial code), compiler is able to optimize your code to get the best of it even when using a local variable.

Few more notes about overall design: you do not validate your parameters anywhere, if they're only used internally to your assembly then mark them internal or private (as appropriate) and ASSERT about their content (for example: why do you declare Token as public?)



Names sound little bit strange, you do not need to repeat the type in the name: TokenSample and TokenValue inside the Token class (and StringValue for a value of type String).



If you intend to fill Token programmatically then adding a Token(string, string) ctor may help to keep your code shorter.







share|improve this answer















share|improve this answer



share|improve this answer








edited Jul 13 at 8:51


























answered Jul 12 at 11:44









Adriano Repetti

9,41611440




9,41611440











  • Thank you for you answer @AdrianoRepetti I have find answer by t3chb0t more suitable :) Thank you for your time.
    – Kiran Shahi
    Jul 13 at 9:03











  • @KiranShahi you welcome; unless you strictly need ordinal comparison then t3's answer should perform really well and it's much more readable.
    – Adriano Repetti
    Jul 13 at 14:55
















  • Thank you for you answer @AdrianoRepetti I have find answer by t3chb0t more suitable :) Thank you for your time.
    – Kiran Shahi
    Jul 13 at 9:03











  • @KiranShahi you welcome; unless you strictly need ordinal comparison then t3's answer should perform really well and it's much more readable.
    – Adriano Repetti
    Jul 13 at 14:55















Thank you for you answer @AdrianoRepetti I have find answer by t3chb0t more suitable :) Thank you for your time.
– Kiran Shahi
Jul 13 at 9:03





Thank you for you answer @AdrianoRepetti I have find answer by t3chb0t more suitable :) Thank you for your time.
– Kiran Shahi
Jul 13 at 9:03













@KiranShahi you welcome; unless you strictly need ordinal comparison then t3's answer should perform really well and it's much more readable.
– Adriano Repetti
Jul 13 at 14:55




@KiranShahi you welcome; unless you strictly need ordinal comparison then t3's answer should perform really well and it's much more readable.
– Adriano Repetti
Jul 13 at 14:55












 

draft saved


draft discarded


























 


draft saved


draft discarded














StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f198350%2freplace-all-the-occurrence-of-a-string%23new-answer', 'question_page');

);

Post as a guest













































































Popular posts from this blog

Chat program with C++ and SFML

Function to Return a JSON Like Objects Using VBA Collections and Arrays

Will my employers contract hold up in court?