Is it really bad to perform two different tasks in one cycle?

Question

I am working on a highly specialized search engine for my database. When a user submits a search request, the mechanism breaks the search terms into an array and passes through it. Within the loop, each search query is checked for several possible scenarios to determine what this may mean. When the search term matches the script, the WHERE clause is added to the SQL query. Some terms can have several meanings, and in these cases the engine creates a list of sentences that help the user to narrow down the results.

In addition: in case someone is interested in learning, ambiguous terms are specified by prefixing them with a keyword. For example, 1954 may be a year or a serial number. The engine will offer the user both of these scenarios and change the search term to any year: 1954 or serial number: 1954.

Building an SQL query and refining the sentences in one loop somehow feels wrong for me, but more overhead will be added to separate them, because I have to skip the same array twice and test the same scripts twice. What is the best course of action?

Answer 1

I would probably define two actions in my own functions. Then you will have

foreach (term in terms) { doThing1(); doThing2(); } 

which is good and clean.

Answer 2

Not. That's not bad. I would have thought that a loop would be more complicated twice.

Perhaps some of the tasks can be included in the functions if the tasks are sufficiently decoupled from each other.

Answer 3

I don’t think it makes sense to add a few cycles for the sake of theoretical purity, especially considering that if you are going to add a cycle against many scenarios, you go from O (n) → O (n * # scripts). Another way to break this without falling into the “Method of God” trap is to have a method that starts one loop and returns an array of matches, and another that searches for each item in the array of matches.

Answer 4

Using the same cycle for me as the right optimization, try to keep the independent code of the two tasks, so if necessary, you can optimize this optimization.

Answer 5

Your script matches the builder pattern , and if each operation is quite complex , then it will be useful for you to understand a little. This is engineering waaaaaay, if all your logic fits into 50 lines of code, but if you have dependencies for control and complex logic, then you should use a proven template to solve the separation problem. It might look like this:

 var relatedTermsBuilder = new RelatedTermsBuilder(); var whereClauseBuilder = new WhereClauseBuilder(); var compositeBuilder = new CompositeBuilder() .Add(relatedTermsBuilder) .Add(whereClauseBuilder); var parser = new SearchTermParser(compositeBuilder); parser.Execute("the search phrase"); string[] related = relatedTermsBuilder.Result; string whereClause = whereClauseBuilder.Result; 

Helper objects will look like this:

 public interface ISearchTermBuilder { void Build(string term); } public class SearchTermParser { private readonly ISearchTermBuilder builder; public SearchTermParser(ISearchTermBuilder builder) { this.builder = builder; } public void Execute(string phrase) { foreach (var term in Parse(phrase)) { builder.Build(term); } } private static IEnumerable<string> Parse(string phrase) { throw new NotImplementedException(); } } 

Answer 6

I do not consider it wrong to do two actions in one cycle. I even suggest making two methods that are called from the loop, for example:

 for (...) { refineSuggestions(..) buildQuery(); } 

On the other hand, O (n) = O (2n)

So don’t worry too much - it’s not such a great sin.

Answer 7

I would call it a code smell, but not very bad. I would highlight the functionality inside the loop by putting one of the first things, and then after the empty line and / or commenting on the other.

Answer 8

I would look at it as if it were an instance of the observer pattern: every time you loop, you raise an event, and as many observers as you want, you can subscribe to it. Of course, it would be superfluous to do this as a template, but the similarities tell me that just two or three or how many actions you want is just fine.

Answer 9

You could run two loops.

If this is a lot, this is business logic, you can create some data structure in the first loop, and then use it to generate SQL, something like

 search_objects = [] loop through term in terms search_object = {} search_object.string = term // suggestion & rules code search_object.suggestion = suggestion search_object.rule = { 'contains', 'term' } search_objects.push(search_object) loop through search_object in search_objects //generate SQL based on search_object.rule 

This, at the very least, saves you the trouble of having to do if / then / elses in both loops, and I think it’s a little easier to move the creation of SQL code outside the first loop.

Answer 10

If the things you do in the loop are connected, then that's fine. It probably makes sense to code “material for each iteration” and then wrap it in a loop, as this is possible, as you think about it in your head.

Add a comment, and if it is too long, look at its separation or using simple utility methods.

Answer 11

I think it can be argued that this may not be quite an agnostic language; it also depends heavily on what you are trying to accomplish. If you set several tasks in a loop in such a way that they cannot be easily parallelized by the compiler for a parallel environment, then this is definitely the smell of code.