1
\$\begingroup\$

The brief is to be able to generate links to artwork files and PDFs easily and quickly without having to use a browser.

I have added an image box which shows the user the image upon clicking one. The links are taken from a dir listing in Google Drive. There is then some conversion to change the folder location to the Google Drive URL.

I would love a review and any pointers to better ways to do this.

Imports System.IO
Imports System.Reflection
Imports System.Net
Imports System.Text
Public Class Form1
 Dim MyArray() As String
 Dim FolderDriveList() As String
 Dim FolderWebList() As String
 Private Sub Form1_Load(sender As Object, e As EventArgs) Handles MyBase.Load
 'Load base data into Listbox1
 Using sr As StreamReader = New StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream("AppName.Products.txt"))
 MyArray = Split(sr.ReadToEnd(), vbLf)
 End Using
 ListBox1.DataSource = MyArray
 Using sr As StreamReader = New StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream("AppName.GoogleDriveLinks.txt"))
 FolderDriveList = Split(sr.ReadToEnd(), vbLf)
 End Using
 Using sr As StreamReader = New StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream("AppName.GoogleDriveWebLinks.txt"))
 FolderWebList = Split(sr.ReadToEnd(), vbLf)
 End Using
 End Sub
 Private Sub ListBox1_Click(sender As Object, e As EventArgs) Handles ListBox1.Click
 'Select item in list, update image if preview is enabled
 Dim SelectedFile As String, X As Long
 SelectedFile = TextBox2.Text
 For X = LBound(FolderDriveList) To UBound(FolderDriveList)
 SelectedFile = Replace(SelectedFile, FolderDriveList(X).Trim, FolderWebList(X).Trim & "/")
 Next
 If CheckBox1.Checked = True Then
 LoadWebImageToPictureBox(ImagePreview, SelectedFile)
 Else
 ImagePreview.Image = Nothing
 End If
 End Sub
 Private Sub CheckBox1_CheckedChanged(sender As Object, e As EventArgs) Handles CheckBox1.CheckedChanged
 'Turn the image preview on and off
 Dim SelectedFile As String, X As Long
 SelectedFile = TextBox2.Text
 For X = LBound(FolderDriveList) To UBound(FolderDriveList)
 SelectedFile = Replace(SelectedFile, FolderDriveList(X).Trim, FolderWebList(X).Trim & "/")
 Next
 If CheckBox1.Checked = True Then
 LoadWebImageToPictureBox(ImagePreview, SelectedFile)
 Else
 ImagePreview.Image = Nothing
 End If
 End Sub
 Private Sub ListBox1_DoubleClick(sender As Object, e As EventArgs) Handles ListBox1.DoubleClick
 'Push SelectedFile across to the Link List Box
 Dim SelectedFile As String, X As Long
 SelectedFile = TextBox2.Text
 For X = LBound(FolderDriveList) To UBound(FolderDriveList)
 SelectedFile = Replace(SelectedFile, FolderDriveList(X).Trim, FolderWebList(X).Trim & "/")
 Next
 ListBox2.Items.Add(SelectedFile)
 End Sub
 Private Sub ListBox2_DoubleClick(sender As Object, e As EventArgs) Handles ListBox2.DoubleClick
 'Remove Selected Item from Copy List
 ListBox2.Items.Remove(ListBox2.SelectedItem)
 End Sub
 Private Sub ListBox1_SelectedIndexChanged(sender As Object, e As EventArgs) Handles ListBox1.SelectedIndexChanged
 'Populate Textbox2 with single item when selected from Listbox1
 TextBox2.Text = ListBox1.SelectedValue
 End Sub
 Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
 'Copy single item from Textbox2 to clipboard
 Dim SelectedFile As String, X As Long
 SelectedFile = TextBox2.Text
 For X = LBound(FolderDriveList) To UBound(FolderDriveList)
 SelectedFile = Replace(SelectedFile, FolderDriveList(X).Trim, FolderWebList(X).Trim & "/")
 Next
 Clipboard.Clear()
 Clipboard.SetText(SelectedFile)
 End Sub
 Private Sub Button2_Click(sender As Object, e As EventArgs) Handles Button2.Click
 'Copy all items from Listbox2 to clipboard
 Dim CopyString As String
 CopyString = ""
 For Each ListBoxLine As Object In ListBox2.Items
 CopyString = CopyString & ListBoxLine.ToString & vbLf
 Next
 Clipboard.Clear()
 Clipboard.SetText(CopyString)
 ListBox2.Items.Clear()
 End Sub
 Private Sub TextBox1_TextChanged(sender As Object, e As EventArgs) Handles TextBox1.TextChanged
 'Update Listbox1 from Textbox1 (using for filtering)
 Dim FilteredArray(0) As String
 Dim ProdName As String
 Dim X As Long = 0
 ListBox1.DataSource = MyArray
 For Each ProdName In ListBox1.Items
 If InStr(UCase(ProdName), UCase(TextBox1.Text)) > 0 Then
 ReDim Preserve FilteredArray(X)
 FilteredArray(X) = ProdName
 X = X + 1
 End If
 Next
 ListBox1.DataSource = FilteredArray
 End Sub
 Public Function LoadWebImageToPictureBox(ByVal pb As PictureBox, ByVal ImageURL As String) As Boolean
 'Load preview image
 Dim objImage As MemoryStream
 Dim objwebClient As WebClient
 Dim sURL As String = Trim(ImageURL)
 Dim bAns As Boolean
 Try
 objwebClient = New WebClient()
 objwebClient.Proxy = Nothing
 objImage = New _
 MemoryStream(objwebClient.DownloadData(sURL))
 pb.Image = Image.FromStream(objImage)
 bAns = True
 Catch ex As Exception
 bAns = False
 End Try
 Return bAns
 End Function
End Class

Sample Data:

Products.txt:

brochures\australbricks\AB-Bricks-BowralBricks-NAT.jpg
brochures\australbricks\AB-Bricks-BowralBricks-NAT.pdf
brochures\australbricks\AB-Bricks-BrickBrochure-NSW.jpg
brochures\australbricks\AB-Bricks-BrickBrochure-NSW.pdf
brochures\australbricks\AB-Bricks-BrickBrochure-QLD.jpg
brochures\australbricks\AB-Bricks-BrickBrochure-QLD.pdf
brochures\australbricks\AB-Bricks-BrickBrochure-SA.jpg

GoogleDriveLinks.txt:

brochures\australbricks\
brochures\australmasonry\CASESTUDIES\
brochures\australmasonry\CMAA\
brochures\australmasonry\SALE\
brochures\australmasonry\
brochures\australpavers\
brochures\australprecast\
brochures\auswesttimbers\

GoogleDriveWebLinks:

https://googledrive.com/host/0B-ZYtyWU3Ek_ZDRneURkdXprcEk
https://googledrive.com/host/0B-ZYtyWU3Ek_Z3hGSzVvdjVXcGs
https://googledrive.com/host/0B-ZYtyWU3Ek_UnFyRUxyX0FrMXM
https://googledrive.com/host/0B-ZYtyWU3Ek_NFVBRzZ4MHZGN0E
https://googledrive.com/host/0B-ZYtyWU3Ek_WHpsWUlidWVCdXM
https://googledrive.com/host/0B-ZYtyWU3Ek_SHZzNUZtdGNlZlE
https://googledrive.com/host/0B-ZYtyWU3Ek_WmRWVXpqRzVPbnM
https://googledrive.com/host/0B-ZYtyWU3Ek_d3hFUmpZV2RHaTA
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Apr 17, 2015 at 2:33
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$
  1. First of all I would like to give you some general hints for refactoring your code for better code quality (in my opinion):

    • Name your variables in a meaningful way - also the controls: Button1 isn't a good name.
    • When declaring and also instantiating a variable the Type = is superfluous: Dim sr as new StreamReader instead of Dim sr as StreamReader = new StreamReader

    • Better use method names instead of comments

    • Try to extract code pieces to methods instead of doing serveral things in one EventHandler.

Your Form1_Load-Method could look like this:

Private Sub Form1_Load(sender As Object, e As EventArgs) Handles MyBase.Load
 LoadBaseDataIntoListbox1()
 LoadFolderDriveList()
 LoadFolderWebList()
End Sub
  • Try not to have copies of snippets in your code. Again: Try to extract methods:

The following code is found several times in you project:

For X = LBound(FolderDriveList) To UBound(FolderDriveList)
 SelectedFile = Replace(SelectedFile, FolderDriveList(X).Trim, FolderWebList(X).Trim & "/")
Next
If CheckBox1.Checked Then
 LoadWebImageToPictureBox(ImagePreview, SelectedFile)
Else
 ImagePreview.Image = Nothing
End If

Also I have some suggestions for improvement of this Method:

'This is VB6 style
'For X = LBound(FolderDriveList) To UBound(FolderDriveList)
' SelectedFile = Replace(SelectedFile, FolderDriveList(X).Trim, FolderWebList(X).Trim & "/")
'Next
'Use this instead:
For i = 0 To FolderDriveList.Length - 1
 SelectedFile = SelectedFile.Replace(FolderDriveList(i).Trim, FolderWebList(i).Trim & "/")
Next

For loading text from resources I personally would prefer to use Resources instead of the hard coded Resource-Name:

Private Sub LoadBaseDataIntoListbox1()
 Using sr As New StringReader(My.Resources.Resource.Products)
 'I recommend not to use VB6-Like syntax:
 ' String.Split(separator) instead of Split(String, separator)
 ' Environment.NewLine insetad of old VisualBasic constant vblf
 Products = sr.ReadToEnd.Split(Environment.NewLine)
 End Using
 ProductsBox.DataSource = Products
End Sub

Always use using when using Disposable objects you do not need afterwards:

Public Function LoadWebImageToPictureBox(ByVal pb As PictureBox, ByVal ImageURL As String) As Boolean
 'Load preview image
 Try
 Using objwebClient As New WebClient()
 objwebClient.Proxy = Nothing
 Dim sURL As String = ImageURL.Trim
 Using objImage As New MemoryStream(objwebClient.DownloadData(sURL))
 pb.Image = Image.FromStream(objImage)
 End Using
 End Using
 Return True
 Catch ex As Exception
 Return False
 End Try
End Function
  1. Now to the functionality of the loading process:

What are you trying to do with this code snippet?

For X = LBound(FolderDriveList) To UBound(FolderDriveList)
 SelectedFile = Replace(SelectedFile, FolderDriveList(X).Trim, FolderWebList(X).Trim & "/")
Next

It would be helpful, if you could give us an example of the content of your resource lists.

I think it would be nice to create a new class which represents an object with the different link types. You would be able to bind a list of those objects to your controls and define which Property (link type) to display.

answered Apr 30, 2015 at 11:46
\$\endgroup\$
2
  • \$\begingroup\$ What I would love this thing to do is read the JPG and PDF contents of the site at run time and do away with all 3 text file objects but I don't think i can poll a site for a list of files and I don't think Code Review is the place for me to start collaborating on such a change. \$\endgroup\$ Commented Apr 30, 2015 at 22:41
  • \$\begingroup\$ Now I understand what the goal of your post was. Yes, this is not the right place to ask this I think. A little hint: Maybe the Google Drive API is what you are looking for. link \$\endgroup\$ Commented May 3, 2015 at 22:06

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.