Barcode (Code 128)

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("", 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))) 'Gro

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 Select 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.

StackExchange Code Review Q#135292, answer score: 3