Sign out for = spaghetti code?

Question

I've always been taught to have one way out of functions and write code that doesn't skip everywhere, is the next bad code, or is there a better way to write a for loop without requiring an "exit for",

dim dt as datatable = FillDataTableFunction() dim bWrite as boolean = false for each row as datarow in dt.rows if row.item("somecolumn") <> string.empty then bWrite = true exit for end if next if bWrite then 'do stuff end if 

I think I just think that this will reduce unnecessary iterations through the for loop, but for some reason this seems like bad coding practice.

Answer 1

"I was always taught" - at some point in life, people begin to learn, not learn :-)

Do not accept anything like the gospel (even from me - if you do not agree, make your own mind). Check out the guidelines to see why they exist.

The reason you were taught that multiple exit points are bad is because they often lead to code that is difficult to execute. In other words, a 400-line function packed with return is difficult to analyze in terms of its behavior.

Your little piece of code does not suffer from this. Next guide: if you see the control flow on one screen in the editor, this is normal. And, since 12 lines will fit into any editor I've used over the past two decades, your code is very readable.

In fact, I saw code from “never use multiple exit points” that is much less readable than the one that will be created by breaking their rules. Usually it includes multi-level while statements, so confusing that they have to break them down into several lines and continue to be painfully parsed.

Aim for readability. If recommendations help with this, use them. If not, throw them out the window.

Answer 2

What you call a “spaghetti code” is the use of the old “goto” keyword that was used in the past. (e.g. Assembly, GwBasic, etc.).

Using gaps to exit cycle structures and decision structures is completely natural.

Answer 3

Your code snippet is not so bad.

This is probably more a matter of taste, but I would put this in a separate function:

 Public Function ShouldWrite(ByVal dt As DataTable) For Each row As DataRow In dt.Rows If row.Item("somecolumn") <> String.Empty Then Return True End If Next Return False End Function 

Or use LINQ:

  Dim bWrite As Boolean = dt.AsEnumerable().Any(Function(row) row.Item("somecolumn") <> String.Empty) 

Answer 4

I understand that this was also bad practice. However, I do not think this is considered bad practice. In fact, I can read your code easily: this is the end of well-written programs, I think ...

However, there is one more step that everyone can change to improve their code: in a loop, you try to find the first line with an element that matches a certain condition. If you are programming a more “functional” way (I don’t know what specific VB objects are in this regard, but this is still a good solution), let's say:

 bWrite = rows.exist(r | r.item("somecolumn") <> string.empty) 

Notice how the whole cycle is condensed in the schematic description of what you are looking for. This "way of thinking" I consider it much more important than if you use exit or a difficult condition ...

Answer 5

Having only one way out of your function is more a guideline than a tough and quick rule - do whatever you prefer. Personally, I prefer to return things as soon as possible.

The code you inserted looks very good to me, and not spaghetti-like at all - you short-circuit your loop to avoid having to iterate over 100,000 lines if you find your answer in the first 100. In this case, I would say that Your Exit For is fully justified.

Answer 6

There is more to it than just if a particular team is good or bad. It is very dependent on the situation.

In a simple piece of code, such as your example, it is probably more readable to use output than trying to put it in loop logic. Since the code exiting the loop is so close to where it ends, its spaghettism has such little effect that it is overloaded due to how much more complex the loop should have done the same.

If there is a lot more code, it can be difficult to follow where the output will actually be, and what effect will have for the rest of the code, so in this case it might make sense to try to save one exit point.

Instructions such as exit tend to inherit spaghetti, as they make the transition to another point in the code, but if the jump is close, it will not interrupt the stream more than, for example, the if statement.

Another thing to consider is how often a particular command is used. If a particular design is often used, it is more likely that it will be understood.

Answer 7

Go to the considered malware in modern code looks like this.

 def func: while stuff: ... return False ... end return true end 

In the above, you exit the function all together from the loop. Thus, there are two exit points, one of which cannot seem like this. The return is "hidden" in other lines of the loop and may be missed by the reader.

Answer 8

The solution is socially acceptable, since it actually leaves a for loop at the bottom. It is important to make the code understandable, especially for you after a few months when you forget what you did today. If you get used to using Exit For, it will most likely be easier to recognize at a glance than any other method to exit the loop.

Answer 9

Many “rules” of languages ​​(whether human or computer languages) exist because people who had a sense of what seemed good and who seemed reluctant wanted to help others write well, and therefore tried to determine what made “good "material other than" bad "material. This is not compliance (or lack) of the rule that does something good (or bad). Rather, it is the fact that the things that follow the rule (we hope) are better than the ones that don’t, which led to the writing of the rule in the first place.

I am not terribly keen on "Exit for"; I tend to think that in most cases, when a for-next loop would be in the early stages of exit, some other type of loop would be better. However, there are places where the early exit of the For-Next cycle "feels" right, and therefore I will use it. I am more likely to end the for-next loop early if the expectation is that the loop will end, but the exception for the case with early exit will not be correct.

Answer 10

This is the code I got to work.

 bool someboolean = dt.AsEnumerable().Any(row => row.Field<string>("RowName") != "")