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
1 Answer 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 ofDim 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.
- Name your variables in a meaningful way - also the controls:
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
- 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.
-
\$\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– Dan Donoghue2015年04月30日 22:41:41 +00:00Commented Apr 30, 2015 at 22:41
-