LabVIEW
I need your Feedback for a VI
Step 2 can be replaced with a simple Search And Replace String. There is an input on the top to "Replace All". Set that to TRUE. And make sure the Replace String is an empty string.
I think we have already beat that Space Generator do death already, so you might want to go ahead and replace that.
@AldhairGarza wrote:
Thank you! Could somebody please review the another VI ? 🙂
(Tim already mentioned a few things)
- The "information" part belongs into a diagram disable structure, in fact it could just be a diagram comment. No need for code and indicators.
- Again, the "space available" constants need to be I32. They are integers.
- If you divide using "quotient and remainder" function instead, it will operate on integers and the "floor" is the rounded down value you need. This also eliminates the "round towards -inf" and "To I32". Much cleaner and all blue!
- Yes, use "search and replace string (replace all=True!)". No step 2 loop needed.
- Step 5 could be replaced by "trim whitespace".
- Same for step 3
- step 4. There is "=0" primitive. Instead of the case structure, use a "select" and either concatenate the whitespace or an empty string. no need for a case structure.
- etc. ...
Wow how silly I am. I really appreciate your help thank you very much.
The mentioned Step does not eliminate whitespace, it just checks where was the last one so when the "Replace Substring" extracts a line of text, it doesn't cut a word in half.
I did all the rest you said, but are you sure that it is better to replace the Case Structure for a Select ?
Because with the Case Structure it was a bit cleaner, but I'm just nobody, I don't really know.
This was the final result. If you have any further observations, i'd be glad to read them.
@AldhairGarza wrote:The mentioned Step does not eliminate whitespace, it just checks where was the last one so when the "Replace Substring" extracts a line of text, it doesn't cut a word in half.
Well, I did not really study the code, just looked at the diagram label. Maybe you should rewrite the label and explain better what the loop actually does. 😄
@AldhairGarza wrote:
I did all the rest you said, but are you sure that it is better to replace the Case Structure for a Select ?
Because with the Case Structure it was a bit cleaner, but I'm just nobody, I don't really know.
"Cleaner" is subjective. With the case structure you had to duplicate the "concatenate strings" and operate on several input tunnels. We also cannot immediately tell what's in the other case by looking at the image. The "select" switches exactly one input to the next operation. Also, since it is just a concatenation, I probably would not use a formatting statement.
@AldhairGarza wrote:
This was the final result. If you have any further observations, i'd be glad to read them.
Nothing is ever "final". 😄 I might have a look later.
Not sure if I have time today, but I have the feeling that the two FOR loops could be combined into one.
Yes it's possible! I was thinking about that while I was creating the VI for the first time but didn't retake the Idea later.
Move your space generation to be outside of the loop. That is something that should only need calculated once, so no need to do it every iteration of the loop.
