10
\$\begingroup\$

I wrote an algorithm which should cut a companies name into 3 strings.

Input: 1 String

Output: 3 String.

Conditions:

String 1 2 and 3 shall not be longer then 35 signs. If the Input string is longer then it should be cut to a length of 105.

If you have fun and be interested in algorithms it would be nice if you take a look at it.

 Public Sub CompanyCut()
 //3 output Strings
 Dim var1 As String = ""
 Dim var2 As String = ""
 Dim var3 As String = ""
 If Module1.insertdict("company").Length > 35 Then
 Dim s1 As String = Module1.insertdict("company").ToString
 If s1.Length > 105 Then
 s1 = Microsoft.VisualBasic.Left(s1, 105)
 End If
 //Split String into array at every Whitespace
 Dim pattern As String = "\s"
 Dim sa() As String = Regex.Split(s1, pattern)
 //Variables for loop
 Dim i As Integer = 0
 Dim varyn As Boolean = True
 Dim varyn1 As Boolean = False
 Dim varyn2 As Boolean = False
 //loop which fills var1 var2 and var3 with arrayfields untill size 35
 would be reached
 For i = 0 To sa.Length - 1
 If var1.Length < 35 AndAlso varyn = True Then
 If var1.Length + 1 + sa(i).Length < 35 Then
 var1 = var1 + " " + sa(i).ToString
 Else
 varyn = False
 varyn1 = True
 varyn2 = False
 End If
 End If
 If var2.Length < 35 AndAlso varyn1 = True Then
 If var2.Length + 1 + sa(i).Length < 35 Then
 var2 = var2 + " " + sa(i).ToString
 Else
 varyn1 = False
 varyn = False
 varyn2 = True
 End If
 End If
 If var3.Length < 35 AndAlso varyn2 = True Then
 If var3.Length + 1 + sa(i).Length < 35 Then
 var3 = var3 + " " + sa(i).ToString
 Else
 varyn2 = False
 End If
 End If
 Next
 //my idea was that if it has the same or bigger length all fields must
 be in + the whitespaces
 If var1.Length + var2.Length + var3.Length >= s1.Length Then
 Module1.insertdict("Firma") = var1
 Module1.insertdict("Name2") = var2
 Module1.insertdict("Name3") = var3
 Else
 //this occurs when the string is smaller as 105 signs but not all 
 fields of the array could be placed in the variables.
 Module1.insertdict("Failure") = "Company name need to split by user"
 End If
 Else
 Module1.insertdict("Name2") = ""
 Module1.insertdict("Name3") = ""
 End If
End Sub

**Edit: I forgot to bring up in the Conditions that if it is possible it should be cut at>the whitespace.

What to do now? shall I open a new Post?**

My main problem is to find a sub algorithmn which handles this case

Else
 //this occurs when the string is smaller as 105 signs but not all 
 fields of the array could be placed in the variables.
 Module1.insertdict("Failure") = "Company name need to split by user"
End If

An easy way would be to cut it with substring like in the answers. a more elegant way would be to cut the first 35 chars for the first variable and then trying to cut at whitespaces. If it doesnt works also cut the second 35 for var2. and the rest is going to var3 :)

asked Jul 3, 2014 at 13:42
\$\endgroup\$
3
  • \$\begingroup\$ Your solution attempts to split at whitespace. This is a nice idea, but not explicit in the requirement (and causes many problematic special cases). (Actually, the requirement that the concatenation of the output strings is the input string is also missing from the problem statement) \$\endgroup\$ Commented Jul 3, 2014 at 15:13
  • 1
    \$\begingroup\$ VB uses apostrophes (') to begin single-line comments, not // \$\endgroup\$ Commented Jul 3, 2014 at 18:10
  • 1
    \$\begingroup\$ Check out word wrap algorithms. This is a commonly solved problem in text editor. \$\endgroup\$ Commented Jul 4, 2014 at 10:06

4 Answers 4

7
\$\begingroup\$

Just for the fun of it, here's a one-liner using LINQ.

Dim names = (From i In {0, 35, 70} Where i < input.Length Select input.Substring(i, Math.Min(35, (input.Length - i))))

Example

Dim names As String() = (From i In {0, 35, 70} Where i < input.Length Select input.Substring(i, Math.Min(35, (input.Length - i)))).ToArray()
Dim var1 As String = If((names.Length > 0), names(0), String.Empty)
Dim var2 As String = If((names.Length > 1), names(1), String.Empty)
Dim var3 As String = If((names.Length > 2), names(2), String.Empty)
answered Jul 3, 2014 at 16:56
\$\endgroup\$
0
6
\$\begingroup\$

You don't need to loop. The function SubString and the property Length is enought to handle what you want.

 var1 = companyName
 var2 = ""
 var3 = ""
 If var1.Length > 35 Then
 var2 = var1.Substring(35)
 var1 = var1.Substring(0, 35)
 If var2.Length > 35 Then
 var3 = var2.Substring(35)
 var2 = var2.Substring(0, 35)
 If var3.Length > 35 Then
 var3 = var3.Substring(0, 35)
 End If
 End If
 End If

This could be even smaller by using arrays

 Dim vars(3) As String
 vars(0) = companyName
 For i As Integer = 0 To 2
 If vars(i).Length > 35 Then
 vars(i + 1) = vars(i).Substring(35)
 vars(i) = vars(i).Substring(0, 35)
 Else
 Exit For
 End If
 Next
answered Jul 3, 2014 at 14:08
\$\endgroup\$
4
  • \$\begingroup\$ you should get rid of the Else statement and the Exit For there is no need for it whatsoever \$\endgroup\$ Commented Jul 3, 2014 at 15:06
  • \$\begingroup\$ @Malachi: No, you do need that. If the company name input string is 70 characters or less, you only need to fill indexes 0 and 1 because there will be no string for the third index. Thus you want to short circuit when i is 2. \$\endgroup\$ Commented Jul 3, 2014 at 22:16
  • \$\begingroup\$ vars(i + 1) = vars(i).Substring(35) Do I see something wrong, or did you miss to pass var(i+1) the string from (35,endofstring) \$\endgroup\$ Commented Jul 4, 2014 at 6:51
  • 2
    \$\begingroup\$ @McDauly If you look at the documentation for Substring, the function goes to endofstring if only one parameter is passed. \$\endgroup\$ Commented Jul 4, 2014 at 12:51
6
\$\begingroup\$

Everything in moderation

Whitespace can help make your code nice and readable. However, TOO much whitespace can make your code hard to read. You fall into the latter case: you have way too much whitespace. A single blank line is all that is necessary to distinguish different logical sections.


DRY - Don't Repeat Yourself

Each iteration through your for loop, you get the length of sa(i) 3 times and could have to convert sa(i) to a string at most 3 times. Instead, do the calculations up front and store the results in a variable.


Be consistent

Keep indents on the same level, otherwise you can confuse both yourself and potential readers later on. Currently you have:

//my idea was that if it has the same or bigger length all fields must
 be in + the whitespaces
 If var1.Length + var2.Length + var3.Length >= s1.Length Then
 Module1.insertdict("Firma") = var1
 Module1.insertdict("Name2") = var2
 Module1.insertdict("Name3") = var3
Else
 //this occurs when the string is smaller as 105 signs but not all 
 fields of the array could be placed in the variables.
 Module1.insertdict("Failure") = "Company name need to split by user"
End If

This is confusing: does the Else statement go with another if statement? Do all the Module1.insert... statements belong inside the if statement or in another outside block of code?

Instead, indent consistently which gets this code:

//my idea was that if it has the same or bigger length all fields must
//be in + the whitespaces
If var1.Length + var2.Length + var3.Length >= s1.Length Then
 Module1.insertdict("Firma") = var1
 Module1.insertdict("Name2") = var2
 Module1.insertdict("Name3") = var3
Else
 //this occurs when the string is smaller as 105 signs but not all 
 //fields of the array could be placed in the variables.
 Module1.insertdict("Failure") = "Company name need to split by user"
End If

This single indentation change removed all of the confusion

answered Jul 3, 2014 at 14:22
\$\endgroup\$
1
  • \$\begingroup\$ Sorry I my fail, I never considered that the code is read on "levels". I will do it next time. \$\endgroup\$ Commented Jul 4, 2014 at 6:53
5
\$\begingroup\$

I like the_lotus first block of code, but I think you would want to remove the nested if statements

var1 = companyName
var2 = ""
var3 = ""
If var1.Length > 105 Then
 var3 = var1.Substring(70,105)
 var2 = var1.Substring(35,70)
 var1 = var1.Substring(0,35)
ElseIf var1.Length > 70 Then
 var3 = var1.Substring(70)
 var2 = var1.Substring(35,70)
 var1 = var1.Substring(0,35)
ElseIf var1.Length > 35 Then
 var2 = var1.Substring(35)
 var1 = var1.Substring(0,35)
End If

anything other than these cases will just be var1

So there is no need for nested if statements.


I was looking at the fancy for loop from the_lotus's answer, and I am thinking that my single if block is going to be more efficient and easier to read.

The reason I am saying this, is that there isn't much that is dynamic about this application, it is pretty set in simple rules.

  • 3 variables
    • 35 characters long (each)

Just no need for anything more complex than this.

Although the for loop is shorter (code character wise) it is doing a lot more than this simple if statement.

answered Jul 3, 2014 at 15:04
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.