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
\$\endgroup\$
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.

\$\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\$ – Dan Donoghue Apr 30 '15 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\$ – RamNow May 3 '15 at 22:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.