Skip to main content


Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Modeling » EMF » How to report minor improvements for emf plugin code
How to report minor improvements for emf plugin code [message #417890] Fri, 28 March 2008 03:04 Go to next message
Reinhold Bihler is currently offline Reinhold BihlerFriend
Messages: 64
Registered: July 2009
Member
Hi,

skimming through the EMF code sometimes I find some minor issues
that could be implemented just a bit more elegant (at least I think so).
For example in the getImportedName() method of class
org.eclipse.emf.codegen.ecore.genmodel.impl.GenModelImpl
....
index = qualifiedName.indexOf("$");
String baseName = index == -1 ? qualifiedName : qualifiedName.substring(0,
index);
if (baseName.contains("."))
{
importManager.addImport(index == -1 ? qualifiedName :
qualifiedName.substring(0, index));
return importManager.getImportedName(qualifiedName);
}...

could be written as
....
index = qualifiedName.indexOf("$");
String baseName = index == -1 ? qualifiedName : qualifiedName.substring(0,
index);
if (baseName.contains("."))
{
importManager.addImport(baseName);
return importManager.getImportedName(qualifiedName);
}...

such issues seem just so negligible that they could be ignored as well.
Opening a bug for such stuff seems a bit pedantic. What do you think?

Reinhold

--
www.emf4net.org

Report message to a moderator

Re: How to report minor improvements for emf plugin code [message #417907 is a reply to message #417890] Fri, 28 March 2008 12:25 Go to previous messageGo to next message
Ed Merks is currently offline Ed MerksFriend
Messages: 33141
Registered: July 2009
Senior Member
Reinhold,

Comments below.

Reinhold Bihler wrote:
> Hi,
>
> skimming through the EMF code sometimes I find some minor issues
> that could be implemented just a bit more elegant (at least I think so).
> For example in the getImportedName() method of class
> org.eclipse.emf.codegen.ecore.genmodel.impl.GenModelImpl
> ....
> index = qualifiedName.indexOf("$");
> String baseName = index == -1 ? qualifiedName : qualifiedName.substring(0,
> index);
> if (baseName.contains("."))
> {
> importManager.addImport(index == -1 ? qualifiedName :
> qualifiedName.substring(0, index));
> return importManager.getImportedName(qualifiedName);
> }...
>
> could be written as
> ....
> index = qualifiedName.indexOf("$");
>
I imagine qualifiedName.indexOf('$') would be more efficient as well.
> String baseName = index == -1 ? qualifiedName : qualifiedName.substring(0,
> index);
> if (baseName.contains("."))
>
Even this is likely to be more efficient doing baseName.indexOf('.') != -1.
> {
> importManager.addImport(baseName);
>
Imports are done a lot, so creating a string one less time each time
seems kind of important.
> return importManager.getImportedName(qualifiedName);
> }...
>
> such issues seem just so negligible that they could be ignored as well.
>
I suppose it's likely to be a small issue, but I suspect that imports
are high volume and improvements are improvements.
> Opening a bug for such stuff seems a bit pedantic. What do you think?
>
I've been accused of being pedantic, so I won't mind. I'll categorize
it as an enhancement request and gladly make the improvements.
> Reinhold
>
>

Ed Merks
Professional Support: https://www.macromodeling.com/

Report message to a moderator

Re: How to report minor improvements for emf plugin code [message #417909 is a reply to message #417907] Fri, 28 March 2008 14:28 Go to previous messageGo to next message
Reinhold Bihler is currently offline Reinhold BihlerFriend
Messages: 64
Registered: July 2009
Member
Ed,

>> if (baseName.contains("."))
> Even this is likely to be more efficient doing baseName.indexOf('.')
> != -1.

Sometimes there is a tradeoff between readability and efficiency. In this
case
I would vote for readability... but thats a question of personal preference
and
really not worth discussing ;-)

> I've been accused of being pedantic, so I won't mind. I'll categorize it
> as an enhancement request and gladly make the improvements.

I could not find the enhancement bug when I checke the bugzilla 5 minutes
ago. Should I open it? I would be glad to do so.

In the future I'll open enhancement bugs of such stuff.

Reinhold

--
www.emf4net.org

Report message to a moderator

Re: How to report minor improvements for emf plugin code [message #417910 is a reply to message #417909] Fri, 28 March 2008 14:32 Go to previous message
Ed Merks is currently offline Ed MerksFriend
Messages: 33141
Registered: July 2009
Senior Member
This is a multi-part message in MIME format.
--------------070003030901050709010603
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Reinhold,

One of the bugzilla's severity choices is "enhancement".

Probably way more people will benefit from faster execution than from
slightly more readable code...

Thanks for looking at code very carefully!


Reinhold Bihler wrote:
> Ed,
>
>
>>> if (baseName.contains("."))
>>>
>> Even this is likely to be more efficient doing baseName.indexOf('.')
>> != -1.
>>
>
> Sometimes there is a tradeoff between readability and efficiency. In this
> case
> I would vote for readability... but thats a question of personal preference
> and
> really not worth discussing ;-)
>
>
>> I've been accused of being pedantic, so I won't mind. I'll categorize it
>> as an enhancement request and gladly make the improvements.
>>
>
> I could not find the enhancement bug when I checke the bugzilla 5 minutes
> ago. Should I open it? I would be glad to do so.
>
> In the future I'll open enhancement bugs of such stuff.
>
> Reinhold
>
>


--------------070003030901050709010603
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
Reinhold,<br>
<br>
One of the bugzilla's severity choices is "enhancement".&nbsp;&nbsp; <br>
<br>
Probably way more people will benefit from faster execution than from
slightly more readable code...<br>
<br>
Thanks for looking at code very carefully!<br>
<br>
<br>
Reinhold Bihler wrote:
<blockquote cite="mid:fsiva3$k1u$1@build.eclipse.org" type="cite">
<pre wrap="">Ed,

</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">if (baseName.contains("."))
</pre>
</blockquote>
<pre wrap="">Even this is likely to be more efficient doing baseName.indexOf('.')
!= -1.
</pre>
</blockquote>
<pre wrap=""><!---->
Sometimes there is a tradeoff between readability and efficiency. In this
case
I would vote for readability... but thats a question of personal preference
and
really not worth discussing ;-)

</pre>
<blockquote type="cite">
<pre wrap="">I've been accused of being pedantic, so I won't mind. I'll categorize it
as an enhancement request and gladly make the improvements.
</pre>
</blockquote>
<pre wrap=""><!---->
I could not find the enhancement bug when I checke the bugzilla 5 minutes
ago. Should I open it? I would be glad to do so.

In the future I'll open enhancement bugs of such stuff.

Reinhold

</pre>
</blockquote>
<br>
</body>
</html>

--------------070003030901050709010603--

Ed Merks
Professional Support: https://www.macromodeling.com/

Report message to a moderator

Previous Topic:Re: Teneo resources and the EMF Validation framework do not properly work together
Next Topic:Workspace dependency for RCP GMF-Application?
Goto Forum:
  

[ Syndicate this forum (XML) ] [ RSS ]

Current Time: Fri Apr 26 00:05:11 GMT 2024

Powered by FUDForum. Page generated in 0.03216 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software

Back to the top

Sign up to our Newsletter

A fresh new issue delivered monthly