8
\$\begingroup\$

I wrote the code below to generate Code 128 barcodes, based on the specs.

Public Class UcBarCode    'Inherits UserControl
    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
        PictureBox1.Image = C128bmp("Teste123")
        WebBrowser1.DocumentText = String.Format("<html><body><img alt=""Embedded Image"" src=""data:image/png;base64,{0}"" /></body></html>", C128b64png("Teste123"))
    End Sub

    Function C128b64png(ByVal text As String) As String
        Dim bmp = C128bmp(text), str = New IO.MemoryStream
        bmp.Save(str, Imaging.ImageFormat.Png)
        Return System.Convert.ToBase64String(str.ToArray)
    End Function
    Function C128bmp(ByVal text As String) As Bitmap
        text = String.Concat(text.Select(Function(c) IIf(AscW(c) >= 32 AndAlso AscW(c) <= 126, c, "_"c)))
        Dim bars = C128bars(text), bmp As New Bitmap(bars.Sum, 50), x = 0, clr = Color.White
        For Each bar In bars
            For i = 1 To bar                                                      'As many columns as bar width
                For y = 0 To bmp.Height - 1
                    bmp.SetPixel(x, y, clr)                                       'Colors column from top to bottom
                Next
                x += 1                                                            'Step to next column
            Next
            If clr = Color.White Then clr = Color.Black Else clr = Color.White    'Alternates colors B&W
        Next
        Return bmp
    End Function
    Function C128bars(ByVal text As String) As Integer()
        Dim chars = text.ToCharArray, bw, vl As New List(Of Integer)
        bw.Add(10)                                              'Leading Quiet Zone
        If chars.All(Function(c) "0123456789".Contains(c)) Then
            bw.AddRange(GetBarset(105))                         'Start bars for Code C (digit optimized)
            vl.Add(105)
            Dim pairs As New List(Of String)
            For i = 0 To chars.Count - 1 Step 2
                pairs.Add(String.Concat(chars.Skip(i).Take(2))) 'Groups digits in pairs
            Next
            For Each pair In pairs
                If pair.Length = 2 Then
                    bw.AddRange(GetBarset(CInt(pair)))          'Bars for pair of digits
                    vl.Add(CInt(pair))
                ElseIf pair.Length = 1 Then
                    bw.AddRange(GetBarset(100))                 'Changes to Code B (ASCII optimized)
                    vl.Add(100)
                    bw.AddRange(GetBarset(CInt(pair) + 16))     'Bars for single digit
                    vl.Add(CInt(pair) + 16)
                End If
            Next
        Else
            bw.AddRange(GetBarset(104))                         'Start bars for Code B (ASCII optimized)
            vl.Add(104)
            For Each c In chars
                bw.AddRange(GetBarset(AscW(c) - 32))            'Bars for character
                vl.Add(AscW(c) - 32)
            Next
        End If
        Dim s = 0
        For i = 0 To vl.Count - 1
            s += vl(i) * Math.Max(i, 1)                         'Calculates Checksum
        Next
        Dim r = s Mod 103
        bw.AddRange(GetBarset(r))                               'Bars for checksum character
        bw.AddRange({2, 3, 3, 1, 1, 1, 2, 10})                  'End bars and trailing Quiet Zone
        Return bw.ToArray
    End Function

    Function GetBarset(ByVal code As Integer) As Integer()
        Return barset2.Substring(code * 6, 6).Select(Function(c) Val(c)).ToArray
    End Function

    Const barset2 = "212222222122222221121223121322131222122213122312132212221213221312231212112232122132122231113222123122123221223211221132221231213212223112312131311222321122321221312212322112322211212123212321232121111323131123131321112313132113132311211313231113231311112133112331132131113123113321133121313121211331231131213113213311213131311123311321331121312113312311332111314111221411431111111224111422121124121421141122141221112214112412122114122411142112142211241211221114413111241112134111111242121142121241114212124112124211411212421112421211212141214121412121111143111341131141114113114311411113411311113141114131311141411131211412211214211232"
End Class

Could it be optimized?

\$\endgroup\$
3
\$\begingroup\$

SRP

The class in question should only be responsible for generating a Code 128 barcode, but nevertheless it has code in it which belongs to some kind of UI. You should separate that UI code from the code to generate the barcode.

You class is doing to many things here.


Validation

You public methods don't do any validation of the passed in method arguments. This will lead to exposing implementation details of your class which you should avoid.

For example this

Function C128b64png(ByVal text As String) As String
    Dim bmp = C128bmp(text), str = New IO.MemoryStream
    bmp.Save(str, Imaging.ImageFormat.Png)
    Return System.Convert.ToBase64String(str.ToArray)
End Function  

will produce a stacktrace telling a user of your class that there is a C128bmp() method which uses IEnumerable<TResult> Select<TSource, TResult> method.

While we are at this C128b64png() method:

  • the method name isn't really telling a reader what it is about. After some thinking, which is bad while reading code, the reader will come to the result that it will return a base64 encoded png image containing a Code 128 barcode. Much better would be to let the method return the Bitmap, have an extension method handling the conversation to a png and have an extension method taking a bitmap and returning a base64 representation of that bitmap. This would force the responsibility of the class in question to only creating barcode images, one extension class/module to do the image conversation and one extension class/module to do base64 encoding.

    While we are at thinking about what some code is doing, that is something you want to avoid because if you have a bug and you first need to decypher a piece of code you will lose time and energy. It is much better to write code which could be understood by a different developer at first glance. This leads us to the next point

  • Dim bmp = C128bmp(text), str = New IO.MemoryStream  
    

    declaring multiple variables on the same line makes it super hard to understand the code. Sure one could say, hey there are only two declarations that isn't that hard to read, but this will become a habit like the declarations of C128bmp() method

     Dim bars = C128bars(text), bmp As New Bitmap(bars.Sum, 50), x = 0, clr = Color.White  
    

    which is much harder to understand not only because you are declaring 4 variables but because you use abbreviations for naming things.

  • using local type inference which means that you don't need to state the variables type is a nice to have feature but shouldn't be abused. You should only use this if it is obvious from the right hand side of an assignment which type is used.

    For Dim someValue = "lala" it is clear that someValue is a String, but what about Dim bars = C128bars(text)? Here a reader has to scroll through the code to find the C128bars method to see its returning type. IMO you are abusing that feature.

\$\endgroup\$
  • \$\begingroup\$ There would be much more to say, but I am running out of time. \$\endgroup\$ – Heslacher Jul 19 '16 at 17:00
  • \$\begingroup\$ Thank you, all those critics are welcome, I'll try to stick to those rules, starting by rewriting the code I posted. As for Function C128bmp(ByVal text As String) As Bitmap, nevertheless, I would like to know if such a bitmap can be made-up more efficiently (faster and more resource-saving) than the way I did. Thank you! \$\endgroup\$ – VBobCat Jul 19 '16 at 18:03
  • 1
    \$\begingroup\$ SetPixel is incredible slow. Look into LockBits instead. \$\endgroup\$ – Heslacher Jul 19 '16 at 18: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.