3

I have a requirement for a service that does the following.

Take a block of text and identify the server names in it (by name or ip address). So given:

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec libero felis, accumsan in nunc id, lacinia rutrum libero. Server1 Praesent iaculis consequat est quis elementum. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos Server2 himenaeos. Cras aliquet nisl non tortor interdum semper. Nulla commodo dignissim justo, eu accumsan neque eleifend ut. Etiam malesuada volutpat dolor 192.168.0.2 laoreet placerat. Maecenas posuere ipsum mattis egestas elementum.

The service would return:

  • Server1
  • Server2
  • Server3 (which has ip Address 192.168.0.2)

there are around 7,000 servers and addresses in my DB. So at the moment the only strategy I have is to take the text block as a string and loop through all the servers twice (name and ip) issuing a string.Contains().

Issuing 14,000 Contains seems a bit "brute force". Is there a more elegant way to achieve the same result.

For context this is a rest service running on ASP.Net MVC and C#.

  • 1
    What database are you using? It might be more efficient at searching using SQL than pulling all that into strings. – Adam Zuckerman Apr 18 '16 at 6:14
  • 1
    Given the assumption that you really do need to do something clever and efficient in code, rather than the alternatives the answers/comments suggest, building a "trie" or "prefix tree" out of the server names/ips is the first thing that comes to mind. – Ixrec Apr 18 '16 at 6:59
  • @AdamZuckerman - good idea, hadn't thought of exploring SQL server options. – Lobsterpants Apr 18 '16 at 7:20
  • @lxrec - prefix trees look interesting, off to investigate those :) – Lobsterpants Apr 18 '16 at 7:22
3

If your current code is simple and fast enough for your needs, do nothing. Just to optimize because "it seems a bit brute force" is not a good reason, it will mostly complicate things for no benefit. Do not fall into the trap of premature optimization.

However, if your current code really is too slow for your purposes, first measure where the bottleneck is. Is it really calling 14.000 times string.Contains, or is it selecting the 14.000 server names / Ip addresses from your database? The first issue might be approached by splitting up the text into words which may be potentially a server name, and utilizing a hashset or a more sophisticated data structure. The second issue might be approached by splitting up the text the same way,using the words as a SELECT criteria, assumed your database is properly indexed. The latter one could increase the number of roundtrips, to avoid that, you could implement a stored procedure in your DB, pass the text once over the network and let the SP do the work.

All of these solutions, however, will result in more complicated code than you have now, so make sure this is worth the hassle, otherwise you are probably sacrificing a maintainable solution for useless overcomplication.

|improve this answer|||||
  • All good points about premature optimization and maintainability etc. I know exactly where you are coming from on that one. My worry is not particularly about the performance of this bit of code at the moment, but looking forward to requirements coming down the line it looks like we will need to cope with lots of attributes per server and that would show up weaknesses in the design, so I am just exploring options at the moment. – Lobsterpants Apr 18 '16 at 7:28
  • two good suggestions for splitting &using SQL too. Nice. – Lobsterpants Apr 18 '16 at 7:29
  • 1
    @Lobsterpants: having no optimization prepared is not a weakness in the design - but having unneccessary optimization is. It is typically much easier to evolve simple code for new requirements than "optimized" code. I heavily recommend to optimize your code when the new requirements arrive, only after you implemented them in the most simple possible approach, measured the performance again, and you know for sure your perfomance is bad, not "just in case" – Doc Brown Apr 18 '16 at 9:47
  • I'm not coding anything "optimised" at the moment, just going with the simplest solution. But, should the worst happen, I can at least pop back to this question & know I have some options to fix it. I can see that premature optimisation may be an issue but I can't see that premature knowledge is. – Lobsterpants Apr 18 '16 at 10:14
1

One option would be to use regular expressions: build one regular expression that matches any of the servers (in your case, it would be Server1|Server2|192\.168\.0\.2, you can build that by using something like string.Join("|", servers.Select(Regex.Escape))).

Don't forget about the option to compile the regex, which could speed it up significantly.

|improve this answer|||||

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.