Bug Hunting Session
Bug 52622 - Reduce copy and paste code: lcl_GetImageFromPngUrl
Summary: Reduce copy and paste code: lcl_GetImageFromPngUrl
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: Other All
: medium enhancement
Assignee: José Guilherme Vanz
URL:
Whiteboard: target:4.2.0
Keywords: difficultyBeginner, easyHack, skillCpp
Depends on:
Blocks:
 
Reported: 2012-07-28 16:22 UTC by Thomas Arnhold
Modified: 2016-02-18 16:37 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Arnhold 2012-07-28 16:22:13 UTC
The local method lcl_GetImageFromPngUrl() is implemented five times.

Idea is to make it a helper function and reduce copy and pasted code. Seems that tools/ would be an appropriate directory for this.

Here is the list:

http://opengrok.libreoffice.org/search?q=lcl_GetImageFromPngUrl&project=core
Comment 1 Jorendc 2013-06-28 17:54:50 UTC
NEW right away.
Comment 2 Thomas Arnhold 2013-06-28 19:08:28 UTC
svtools seems to be the more appropriate directory.
Comment 3 Jelle van der Waa 2013-07-12 09:38:20 UTC
Ok, I am willing to work on this bug. But I'm not sure where I should move the method. I guess svtools/source/graphic is the right directory, but I don't think creating a new file for a method is worth it.

So where should I put it ?
Comment 4 Thomas Arnhold 2013-08-02 06:25:18 UTC
@Jelle: You could create a file named helper.cxx or something similar. Then submit it to gerrit and hopefully someone will complain if it's not the right directory.

@mmeeks: Do you have a idea where to put this method in?
Comment 5 Thorsten Behrens (CIB) 2013-09-13 10:40:57 UTC
comphelper seems to be the canonical place to stick those things into.
Comment 6 Julien Nabet 2013-09-14 12:16:33 UTC
Jelle: I put you as assigned to warn clearly other people someone is working on it.
Of course, if you change your mind or lack of time,... don't hesitate to revert:)
Comment 7 Björn Michaelsen 2013-10-04 18:46:09 UTC
adding LibreOffice developer list as CC to unresolved EasyHacks for better visibility.

see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
Comment 8 José Guilherme Vanz 2013-10-19 00:01:14 UTC
I am working in this bug and I have a doubt. I created a new file inside comphelper to move this function.When I compile the code the following problem is showed:

make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/Headers/Library/libsvllo.so <- /home/vanz/git/libo/workdir/unxlngx6.pro/Headers/Library/libcomphelper.so dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/Headers/Library/libi18nutil.so <- /home/vanz/git/libo/workdir/unxlngx6.pro/Headers/Library/libcomphelper.so dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/Headers/Library/libsotlo.so <- /home/vanz/git/libo/workdir/unxlngx6.pro/Headers/Library/libcomphelper.so dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/Headers/Library/libtllo.so <- /home/vanz/git/libo/workdir/unxlngx6.pro/Headers/Library/libcomphelper.so dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/Headers/Library/libutllo.so <- /home/vanz/git/libo/workdir/unxlngx6.pro/Headers/Library/libcomphelper.so dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/Headers/Library/libvcllo.so <- /home/vanz/git/libo/workdir/unxlngx6.pro/Headers/Library/libcomphelper.so dependency dropped.
make[1]: Circular /home/vanz/git/libo/instdir/unxlngx6.pro/program/libsvllo.so <- /home/vanz/git/libo/workdir/unxlngx6.pro/LinkTarget/Library/libcomphelper.so.exports dependency dropped.
make[1]: Circular /home/vanz/git/libo/instdir/unxlngx6.pro/program/libi18nutil.so <- /home/vanz/git/libo/workdir/unxlngx6.pro/LinkTarget/Library/libcomphelper.so.exports dependency dropped.
make[1]: Circular /home/vanz/git/libo/instdir/unxlngx6.pro/program/libsotlo.so <- /home/vanz/git/libo/workdir/unxlngx6.pro/LinkTarget/Library/libcomphelper.so.exports dependency dropped.
make[1]: Circular /home/vanz/git/libo/instdir/unxlngx6.pro/program/libtllo.so <- /home/vanz/git/libo/workdir/unxlngx6.pro/LinkTarget/Library/libcomphelper.so.exports dependency dropped.
make[1]: Circular /home/vanz/git/libo/instdir/unxlngx6.pro/program/libutllo.so <- /home/vanz/git/libo/workdir/unxlngx6.pro/LinkTarget/Library/libcomphelper.so.exports dependency dropped.
make[1]: Circular /home/vanz/git/libo/instdir/unxlngx6.pro/program/libvcllo.so <- /home/vanz/git/libo/workdir/unxlngx6.pro/LinkTarget/Library/libcomphelper.so.exports dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/ResTarget/vclen-US.res <- /home/vanz/git/libo/workdir/unxlngx6.pro/Executable/rsc.run dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/SrsPartTarget/vcl/source/src/btntext.src <- /home/vanz/git/libo/workdir/unxlngx6.pro/Executable/rsc.run dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/SrsPartTarget/vcl/source/src/helptext.src <- /home/vanz/git/libo/workdir/unxlngx6.pro/Executable/rsc.run dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/SrsPartTarget/vcl/source/src/images.src <- /home/vanz/git/libo/workdir/unxlngx6.pro/Executable/rsc.run dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/SrsPartTarget/vcl/source/src/menu.src <- /home/vanz/git/libo/workdir/unxlngx6.pro/Executable/rsc.run dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/SrsPartTarget/vcl/source/src/print.src <- /home/vanz/git/libo/workdir/unxlngx6.pro/Executable/rsc.run dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/SrsPartTarget/vcl/source/src/stdtext.src <- /home/vanz/git/libo/workdir/unxlngx6.pro/Executable/rsc.run dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/SrsPartTarget/vcl/source/src/throbber.src <- /home/vanz/git/libo/workdir/unxlngx6.pro/Executable/rsc.run dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/SrsPartTarget/vcl/source/src/units.src <- /home/vanz/git/libo/workdir/unxlngx6.pro/Executable/rsc.run dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/SrsPartTarget/vcl/source/src/fpicker.src <- /home/vanz/git/libo/workdir/unxlngx6.pro/Executable/rsc.run dependency dropped.
make[1]: Circular /home/vanz/git/libo/workdir/unxlngx6.pro/SrsPartTarget/vcl/source/edit/textundo.src <- /home/vanz/git/libo/workdir/unxlngx6.pro/Executable/rsc.run dependency dropped.

On my .cxx there are the following includes:
#include <vcl/graph.hxx>
#include <vcl/graphicfilter.hxx>
#include <vcl/image.hxx>
#include <osl/file.hxx>
Comment 9 José Guilherme Vanz 2013-10-19 00:08:00 UTC
correcting... ;)
On my .hxx there are the following includes:
#include <vcl/graph.hxx>
#include <vcl/graphicfilter.hxx>
#include <vcl/image.hxx>
#include <osl/file.hxx>
Comment 10 Tomaz Vajngerl 2013-10-19 06:30:12 UTC
Sorry, but this functionality (a helper or not) should go into vcl. The function is clearly for creation of images and it does not use any problematic dependencies so it should be somewhere near Image.cxx.

When you go to comphelper it says "Helper functionality for implementing UNO components". This function has directly nothing to do with UNO so this is not the right place or we could put anything in here which I hope we don't.
Comment 11 Michael Meeks 2013-10-19 14:33:19 UTC
Hi Jose - thanks so much for working on this ! :-) as Tomaz says - putting it into vcl/ and prolly directly into Image itself - perhaps as a constructor / static create method would be wonderul.
Comment 12 José Guilherme Vanz 2013-10-20 03:11:20 UTC
Ok! I'll submit a patch soon :)
Comment 13 Commit Notification 2013-10-20 12:41:03 UTC
Jose Guilherme Vanz committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=288f0029c69ab0532165877637a146f774d5e740

fdo#52622 - Reduce copy and paste code



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 14 Thomas Arnhold 2013-10-20 13:41:49 UTC
Nice work, José :)
Comment 15 Robinson Tryon (qubit) 2015-12-15 16:47:50 UTC
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyBeginner SkillCpp )
[NinjaEdit]
Comment 16 Robinson Tryon (qubit) 2016-02-18 16:37:04 UTC
Remove LibreOffice Dev List from CC on EasyHacks
(curtailing excessive email to list)
[NinjaEdit]