RomRaider Logo

RomRaider

Open Source ECU Tools
 FAQ •  Register •  Login 

RomRaider

Documentation

Community

Developers

It is currently Tue Dec 23, 2025 11:01 am

All times are UTC - 5 hours [ DST ]





Post new topic Reply to topic  [ 7 posts ] 
Author Message
 Post subject: java: Exceptions are for exceptional situations
PostPosted: Fri Mar 06, 2015 9:44 am 
Offline
Newbie
User avatar

Joined: Thu Mar 05, 2015 10:11 pm
Posts: 9
I wanted to comment on the way code uses exceptions for everything, I believe this is a less then perfect approach.
Let's take an example:
Code:
    private void showTable(Object selectedRow) {
        try{
            if(selectedRow instanceof TableTreeNode) {
                TableTreeNode node = (TableTreeNode) selectedRow;
                if(null != node) {
                    getEditor().displayTable(node.getFrame());
                }
            }
        } catch (NullPointerException ex) {
        }
    }


Empty catch block? That's an alarm. This makes debugging and understanding the code pretty difficult. Since there are potentially multiple places where exception would be thrown, maybe somewhere intentionally and maybe somewhere unintentionally, how would a person developing know the original intent?

Here is an even worse example:
Code:
                try{
                    Object selectedRow = getSelectionPath().getLastPathComponent();
                    showTable(selectedRow);
                    setLastSelectedRom(selectedRow);
                }catch(NullPointerException ex) {
                }


Again - empty catch block. Around multiple methods, each of them also ignoring the exception already. What's the purposes of this catch here? This is confusing and only hides issues. I do not know what was the reason for this approach - maybe someone was in a rush to write the code faster, but I believe this should be improved.

If whoever with push privileges would be interested, I can make some changes in this department - actually I have already made some: https://github.com/RomRaider/RomRaider/pull/55

_________________
http://rusefi.com - open source ECU hardware


Top
 Profile  
 
 Post subject: Re: java: Exceptions are for exceptional situations
PostPosted: Sat Mar 07, 2015 11:27 pm 
Offline
Newbie
User avatar

Joined: Thu Mar 05, 2015 10:11 pm
Posts: 9
Effective Java™ by Joshua Bloch
Image

ObjectCloner.deepCopy would be a perfect case where this principle should be followed.

What do we currently have?
Code:
    public static Object deepCopy(Object obj) throws Exception {
        ObjectOutputStream outStream = null;
        ...
        try {
            ByteArrayOutputStream byteOut = new ByteArrayOutputStream();
            ...
            return inStream.readObject();
        } catch (Exception e) {
            throw (e);
        }
    }

But what exceptions could be thrown by the block? IOException and ClassNotFoundException. We are not expecting these to happen here under normal condition, these would indicate that something fatal is happening and there is no chance to recover from that fatal issue. Thus a better implementation of this method should be
Code:
public static Object deepCopy(Object obj) {
        ObjectInputStream inStream = null;
        ...
        try {
            ...
            return inStream.readObject();
        } catch (IOException e) {
            throw new IllegalStateException(e);
        } catch (ClassNotFoundException e) {
            throw new IllegalStateException(e);
        }
    }


In this improved version we take care of the exceptions right here and not spill the job of taking care of this exception outside of the method.

_________________
http://rusefi.com - open source ECU hardware


Top
 Profile  
 
 Post subject: Re: java: Exceptions are for exceptional situations
PostPosted: Wed Mar 01, 2017 12:42 pm 
Offline
Newbie
User avatar

Joined: Thu Mar 05, 2015 10:11 pm
Posts: 9
Something along the lines of https://github.com/RomRaider/RomRaider/pull/69/ - will be happy to fix more of these

_________________
http://rusefi.com - open source ECU hardware


Top
 Profile  
 
 Post subject: Re: java: Exceptions are for exceptional situations
PostPosted: Sun Mar 05, 2017 11:46 pm 
Offline
Newbie

Joined: Tue Apr 19, 2011 10:22 pm
Posts: 93
Location: Kitchener, Ontario, Canada
Hey,

Yeah, I certainly agree with you that this is just poor programming practice in any language that includes an exception handling feature. Unfortunately many folks have worked on the code over the years so probably the history is gone for this sort of thing.

Dan

_________________
2010 STI w/ built motor, TGV delete, TBE, KW Coil Overs.


Top
 Profile  
 
 Post subject: Re: java: Exceptions are for exceptional situations
PostPosted: Sat Jul 14, 2018 8:07 pm 
Offline
Experienced
User avatar

Joined: Thu Apr 19, 2012 3:44 am
Posts: 385
Hey Danny, FWIW, when I converted the two old and even older SVN repos to Git to help the project into the future, I preserved ALL of the commit history that was available, only inserting a small delta in between during the Git Graft operation needed to join the two disconnected histories. So a git history on the file and/or layered recursive "git blame" (each execution provides an origin commit, which you can then repeat the operation on the parent of) on the lines in question should at least bring up their comments (if any) and the context of the change to aid understanding of intent.

I've not taken a look at this project since Mr PC nuked some of my posts, just logged in today as I receive various "can't get on the forum" messages via FB, however I might have to grab a copy and see how it is these days. Maybe a little more cross platform? Likely still on ant, though, I guess. I tried to get it converted to Maven but some of the dependencies were in the hard basket and needed some code changes to make work, IIRC. Likely still have the branch and notes somewhere, though if there's interest.

_________________
The type of scooby that I most enjoy!


Top
 Profile  
 
 Post subject: Re: java: Exceptions are for exceptional situations
PostPosted: Sat Dec 01, 2018 6:03 pm 
Offline
Newbie

Joined: Tue Nov 27, 2018 7:27 am
Posts: 1
Fearless wrote:
Hey Danny, FWIW, when I converted the two old and even older SVN repos to Git to help the project into the future, I preserved ALL of the commit history that was available, only inserting a small delta in between during the Git Graft operation needed to join the two disconnected histories. So a git history on the file and/or layered recursive "git blame" (each execution provides an origin commit, which you can then repeat the operation on the parent of) on the lines in question should at least bring up their comments (if any) and the context of the change to aid understanding of intent.

I've not taken a look at this project since Mr PC nuked some of my posts, just logged in today as I receive various "can't get on the forum" messages via FB, however I might have to grab a copy and see how it is these days. Maybe a little more cross platform? Likely still on ant, though, I guess. I tried to get it converted to Maven but some of the dependencies were in the hard basket and needed some code changes to make work, IIRC. Likely still have the branch and notes somewhere, though if there's interest.


I would be interested in that branch if you still have it. I've spent the past two days or so looking through this codebase and I would really like to bring it "back to life". From my basic understand as of now, the two main hurdles for updating it are: 1. The native dependencies which have been built for 32bit platforms and 2. Drivers for J2534 interfaces only being 32bit compliant. While I've dug deeper into what fixing the first issue would look like, and have noticed that someone got a 64bit "alpha" driver working for J2534, I still don't have a clear picture of everything that would need to be modified.

From my perspective, getting this to run on the current JVM is pretty important as having a more accessible development environment will, hopefully, attract more contributes. I would say continuing your work to migrate this to maven soon follows that in importance, however if it's nearly complete it's worth just getting out of the way. Also, just to note, I understand this was a topic for discussion before but given that the posts were relatively old (I believe 2013 was when most of the activity ended) I was hoping that the state of some of these dependencies has evolved enough for us to move forward with a bit less headache (may very well not be the case lol).

I guess a more realistic question to ask before going any further would be does RomRaider even need to be updated? While updating code is great, "if it ain't broke don't fix it" may apply here. The only real case I can make for even attempting this at this point would be if there were enough developers interested in contributing who are currently put-off by the dated codebase. A more bias second reason for myself is that I really wanted to use this for my own car and, as a developer by trade, having a robust codebase for the program that could detonate my engine is appealing ^.^

Disclaimer: I understand the tune controls whether or not my engine has issues, but this is a program that controls how the ROM is written to be used with ECUFlash so I still feel it's pretty relevant.

Would love to get some input from you as I've seen you around the forums quite a bit :)


Top
 Profile  
 
 Post subject: Re: java: Exceptions are for exceptional situations
PostPosted: Thu Dec 06, 2018 4:39 pm 
Offline
Newbie

Joined: Tue Jan 05, 2016 11:27 pm
Posts: 64
Mr_Irle wrote:
Fearless wrote:
I guess a more realistic question to ask before going any further would be does RomRaider even need to be updated? While updating code is great, "if it ain't broke don't fix it" may apply here. The only real case I can make for even attempting this at this point would be if there were enough developers interested in contributing who are currently put-off by the dated codebase. A more bias second reason for myself is that I really wanted to use this for my own car and, as a developer by trade, having a robust codebase for the program that could detonate my engine is appealing ^.^

Disclaimer: I understand the tune controls whether or not my engine has issues, but this is a program that controls how the ROM is written to be used with ECUFlash so I still feel it's pretty relevant.

Would love to get some input from you as I've seen you around the forums quite a bit :)


I'm personally heavily biased against Java... if a port to a Python environment using Wx for GUI was in the works, I'd likely be interested in developing :mrgreen:

As you said though, I don't see RomRaider really needing any huge updates. Personally, I never use RomRaider myself for ROM editing. I use RR Logger if I'm laptop logging (though mostly use my tablet and BtSsm for the Legacy) or ECUFlash to modify a ROM.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 7 posts ] 

All times are UTC - 5 hours [ DST ]


Who is online

Users browsing this forum: No registered users and 6 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Jump to:  
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group
Style based on FI Subsilver by phpBBservice.nl