Gfrktyl

I have a problem with my printing program, in which it uses up too much resources while printing (to paper or to .pdf). I try to manually dispose as much as I can, but for example, when I tried printing 300 pages, the program itself used up around 500mb of memory, and I would like to avoid that. When the program finishes printing, it goes back down to 37mb of usage. I use Visual Studio to check performance. Below you can find the code, and any help to manage resources and lower ram usage while printing would be appreciated. I apologize in advance for mixing German and English. And for clarity, this draws n number of each element (one is a string, second an arrow, up or down, third a barcode).

private void DocumentDrucker_PrintPage(object sender, System.Drawing.Printing.PrintPageEventArgs e)

{

    Graphics graphic = e.Graphics;

    SolidBrush brush = new SolidBrush(Color.Black);



    Font font = new Font("Courier New", 27, FontStyle.Bold);



    float pageWidth = e.PageSettings.PrintableArea.Width;

    float pageHeight = e.PageSettings.PrintableArea.Height;



    float fontHeight = font.GetHeight();

    int startX = 40;

    int startY = 40;

    int offsetY = 40;



    for (; elemente < ZumDrucken.Items.Count; elemente++)

    {

        graphic.InterpolationMode = System.Drawing.Drawing2D.InterpolationMode.NearestNeighbor;

        graphic.DrawString(ZumDrucken.Items[elemente].Text, font, brush, startX, startY + offsetY);



        if (ZumDrucken.Items[elemente].Checked == true)

        {

            if (ZumDrucken.Items[elemente].Text.Substring(ZumDrucken.Items[elemente].Text.Length - 1) != "1")

                graphic.DrawImage(Properties.Resources.pfeilU, new Point(Convert.ToInt32(pageWidth * 0.35), offsetY + 37));

            else

                graphic.DrawImage(Properties.Resources.pfeilO, new Point(Convert.ToInt32(pageWidth * 0.35), offsetY + 37));

        }

        else

        {

            if (ZumDrucken.Items[elemente].Text.Substring(ZumDrucken.Items[elemente].Text.Length - 1) != "1")

                graphic.DrawImage(Properties.Resources.pfeilO, new Point(Convert.ToInt32(pageWidth * 0.35), offsetY + 37));

            else

                graphic.DrawImage(Properties.Resources.pfeilU, new Point(Convert.ToInt32(pageWidth * 0.35), offsetY + 37));

        }



        b.Encode(TYPE.CODE128A,ZumDrucken.Items[elemente].Text, Color.Black, Color.Transparent,600,100);

        graphic.DrawImage(b.EncodedImage, new Point(Convert.ToInt32(pageWidth*0.6),offsetY));



        offsetY = offsetY + 175;               



        if (offsetY >= pageWidth-100)

        {

            e.HasMorePages = true;

            offsetY = 0;

            elemente++;



            graphic.Dispose();

            b.Dispose();

            brush.Dispose();

            font.Dispose();



            return;

        }

        else

        {

            e.HasMorePages = false;

        }

    }

    graphic.Dispose();

    b.Dispose();

    brush.Dispose();

    font.Dispose();



}

• don't mix german and english words for names. Best is to stick to english because most/all developers knows the language.


• 
  

  don't repeat yourself. You have some duplicated code which should be removed.

  
  
  

  e.g:

  
  
  

  You create the same Point for printing at 4 different location (new Point(Convert.ToInt32(pageWidth * 0.35), offsetY + 37))

  
  



• don't omit braces {} although they might be optional. Omitting them can lead to hidden and therfore hard to track bugs.

Let's take a look at the if..else construct and how we could refactor it

if (ZumDrucken.Items[elemente].Checked == true)

{

    if ZumDrucken.Items[elemente].Text.Substring(ZumDrucken.Items[elemente].Text.Length - 1) != "1")

        graphic.DrawImage(Properties.Resources.pfeilU, new Point(Convert.ToInt32(pageWidth * 0.35), offsetY + 37));

    else

        graphic.DrawImage(Properties.Resources.pfeilO, new Point(Convert.ToInt32(pageWidth * 0.35), offsetY + 37));

}

else

{

    if (ZumDrucken.Items[elemente].Text.Substring(ZumDrucken.Items[elemente].Text.Length - 1) != "1")

        graphic.DrawImage(Properties.Resources.pfeilO, new Point(Convert.ToInt32(pageWidth * 0.35), offsetY + 37));

    else

        graphic.DrawImage(Properties.Resources.pfeilU, new Point(Convert.ToInt32(pageWidth * 0.35), offsetY + 37));

}  

First we can remove the == true because comparing a non nullable bool to true is senseless. Either it is true or false.

Now let us create a

var imagePoint = new Point(Convert.ToInt32(pageWidth * 0.35), offsetY);  

just before the loop and increase the Y before the if..else we are talking about so the whole thing becomes

imagePoint.Y += 37;

if (ZumDrucken.Items[elemente].Checked)

{

    if (ZumDrucken.Items[elemente].Text.Substring(ZumDrucken.Items[elemente].Text.Length - 1) != "1")

    {

        graphic.DrawImage(Properties.Resources.pfeilU, imagePoint);

    }

    else

    {

        graphic.DrawImage(Properties.Resources.pfeilO, imagePoint);

    }

}

else

{

    if (ZumDrucken.Items[elemente].Text.Substring(ZumDrucken.Items[elemente].Text.Length - 1) != "1")

    {

        graphic.DrawImage(Properties.Resources.pfeilO, imagePoint);

    }

    else

    {

        graphic.DrawImage(Properties.Resources.pfeilU, imagePoint);

    }

}

If we extract the checking of the last character from the text outside of the if..else like so

bool lastCharIsAOne =  ZumDrucken.Items[elemente].Text[ZumDrucken.Items[elemente].Text.Length - 1] == '1';  

we don't create a new string because we just access the char array directly.

But for readability I would introduce a

var currentItem = ZumDrucken.Items[elemente]; 

then the if..else will become

var currentItem = ZumDrucken.Items[elemente]; 

... some more code



bool lastCharIsAOne =  currentItem.Text[currentItem.Text.Length - 1] == '1';  

imagePoint.Y += 37;

if (currentItem.Checked)

{

    if (lastCharIsAOne)

    {

        graphic.DrawImage(Properties.Resources.pfeilU, imagePoint);

    }

    else

    {

        graphic.DrawImage(Properties.Resources.pfeilO, imagePoint);

    }

}

else

{

    if (lastCharIsAOne)

    {

        graphic.DrawImage(Properties.Resources.pfeilO, imagePoint);

    }

    else

    {

        graphic.DrawImage(Properties.Resources.pfeilU, imagePoint);

    }

}  

Now we need to do something about the image which should be printed. We see that pfeilU should be printed if:

(currentItem.Checked && lastCharIsAOne) || (!currentItem.Checked && !lastCharIsAOne) which is the same as (currentItem.Checked == lastCharIsAOne)like @DDrmmr stated in the comments, hence we only need one if and one else which we could reduce to a simple if like so

var currentImage = Properties.Resources.pfeilO;



if (currentItem.Checked == lastCharIsAOne)  

{

    currentImage = Properties.Resources.pfeilU;  

}  

graphic.DrawImage(currentImage, imagePoint);

If we now extract the setting of the graphic.InterpolationMode outside of the loop we will get this

graphic.InterpolationMode = System.Drawing.Drawing2D.InterpolationMode.NearestNeighbor;



var imagePoint = new Point(Convert.ToInt32(pageWidth * 0.35), offsetY);  

var barcodePoint = new Point(Convert.ToInt32(pageWidth * 0.6), 0);  



for (; elemente < ZumDrucken.Items.Count; elemente++)

{

    var currentItem = ZumDrucken.Items[elemente]; 



    graphic.DrawString(currentItem.Text, font, brush, startX, startY + offsetY);



    var currentImage = Properties.Resources.pfeilO;



    bool lastCharIsAOne =  currentItem.Text[currentItem.Text.Length - 1] == '1';  

    if (currentItem.Checked == lastCharIsAOne)  

    {

        currentImage = Properties.Resources.pfeilU;  

    }



    imagePoint.Y += 37;  

    graphic.DrawImage(currentImage, imagePoint);



    b.Encode(TYPE.CODE128A, currentItem.Text, Color.Black, Color.Transparent, 600, 100);



    barcodePoint.Y = offsetY;

    graphic.DrawImage(b.EncodedImage, barcodePoint);



    offsetY = offsetY + 175;  



    ... the other `if..else`

I try to manually dispose as much as I can

This is usually good however this time you dispose too much.

private void DocumentDrucker_PrintPage(object sender, PrintPageEventArgs e)

{

    Graphics graphic = e.Graphics;



    ..



    graphic.Dispose(); // Don't!    

}

You receive the Graphics object via the PrintPageEventArgs. This means that you should not dispose it. The owner takes care of the graphics object. You just use it for drawing. I wonder that this works at all because as soon as the graphics is disposed the DocumentDrucker has nothing to print. This should actually crash.

Do not manually call Dispose(), you will end-up having some duplication (and you're not even disposing those resources in case of errors...). Use using():

using (var graphics = new Graphics())

{

    // Your code here...

}

Also do not create a new Font for each page, reuse the same one for all pages. Maybe it's time to move this code into its own separate class with its instance fields?

You do not need to create a SolidBrush(Color.Black), just use Brush.Black.
Is if (offsetY >= pageWidth-100) correct? You're comparing Y offset with page width.

Note that I suppose elemente and b are instance fields, you might want to make it clear: this.elemente and this.b or rename them to _b and _elemente. Also b isn't best name for an object might it be barcodeImageConverter (guessing from parameters), it's hard to understand what it does without reading the full-code and thinking about it. Also elemente may be something like latestPrintedElement.

Get rid of all those hard-coded constants, they will make your code a nightmare to maintain. Use private const int (or float) for them and you will also gain in readability.

I'd say that memory consumption come from b.Encode() method but we don't have any idea about what it is and what it does. Does it create a bard-code image for the given text? An educate guess may be that you need to manually dispose images generated by b.EncodedImage after you used them and before you create a new one calling Encode():

b.EncodedImage?.Dispose();

b.Encode(TYPE.CODE128A, ...

All this said...to fix memory issues nothing is better than a memory profiler...

It seems that some bitmap is being generated here. It might be causing memory pressure as it is inside a loop.

b.Encode(TYPE.CODE128A,ZumDrucken.Items[elemente].Text, ...

graphic.DrawImage(b.EncodedImage...

Try adding GC.Collect() at the end of event handler and see if consumption of memory is reduced.