T
T
Toseter2012-01-06 05:17:45
Java
Toseter, 2012-01-06 05:17:45

Java Code Review?

Given: there is an open source project, in its infancy. And before continuing, I would like to understand if I messed up somewhere in terms of code and architecture, because there are places in which I very much doubt it.
Actually a question: to whom the person/service will address?
PS link to the project github.com/Toseter/SeleniumRunner/tree/develop
in readme and wiki it is written what kind of project it is and what has already been implemented.

Answer the question

In order to leave comments, you need to log in

3 answer(s)
S
serso, 2012-01-06
@Toseter

Perhaps I'll start harsh criticism:
1. A project under maven is good, but it's bad that it does not support
the standard project directory structure
2. Config.parseConfiguration() - why parse it yourself? For this, there are already written libraries such as Commons CLI for working with the command line and Commons Configuration for storing settings (naturally, this can be replaced with something else, but the message, I think, is clear)
3. Java code conventions need to be supported - the class name isCloneable , package name parserHandlers and

private List<SuiteResult> _results;
not eating well
4.
if ( ... && command != -2)
is hell. What does the constant -2 mean?
5. There is no such word as Formater, you need Formatter
6.
stringBuilder.append("[" + result.getTestSuiteName() + "]\r\n")
- use StringBuilder and for some reason concatenate strings
7.
if (Config.isRunnable() == false)
- do not write like that, use the negation operation.
8. Instead of ParserHelper and static methods, describe the interface - you never know where the input data will have to be parsed from (Of course, use your code for parsing from xml as a default implementation)
9.
 public Command[] getCommands() {
        return commands.toArray(new Command[commands.size()]);
    }
Why not return a list? Do you really need an array somewhere?
ten.
                    } catch (CloneNotSupportedException ex) {
- throwing CloneNotSupportedException - an error of java developers, it is recommended to remove it from the signature from your classes.
I think I'll stop there. In terms of architecture, I strongly recommend looking at how, for example, the jUnit library is implemented

D
Disturbed, 2012-01-06
@Disturbed

from StackOverflow authors :)
codereview.stackexchange.com/

E
evilbloodydemon, 2012-01-06
@evilbloodydemon

maybe ask the developers of selenium?

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question