Sophie 2.0 Process

Тo keep the quality of the product (both internal and external) high, we need to define some additional things (this is related to the SCRUM mandatory definition of done).

This should improve the situation with the following challenges:

• Internal quality (quality of code)
• External quality (quality of the visible product)
• Documentation reading (many things are written, few things are read)
• Not to produce too much process overhead

TODO:

• Task tracking/monitoring
• Personal productivity measuring.

Contents

• 1 Rules

o 1.1 General o 1.2 Discipline o 1.3 Other

• 2 SCRUM
• 3 Task Phases
• 4 Requirements

o 4.1 All Code Tasks

+ 4.1.1 Bad code samples

o 4.2 General

• 5 Roles
• 6 Other notes

o 6.1 Reverting

Rules

This is a list of rules you have to follow. It is not final yet, but disobeying them may lead to penalties.

General

1. Work towards the goal!
   • No matter what this document defines, or how concrete your task is defined, there is no way to define exactly what should be good for our goal, and what should be bad.
   • You have to work towards the goal.
   • When you work on something, you have to understand why and how it is related to our goal.
   • You have to try to drive the whole team towards the goal (no one alone is capable of reaching it)
2. Be honest!
   • You have to present the situation to the leader as is.
   • Even if it does not look good.
   • For example, if you have no idea how to make something, don't tell "almost done" to the leader.
3. Be initiative!
   • If you find a way that, you can make something good to the team, don't wait, but propose it to be done.
4. Look around!
   • If you see problems in the code, in the product, in the organization, or whatever, announce them.
   • Try to keep an eye about what other people do (either for things you can learn from, or for things they are doing bad).
   • Try to get a global vision of what is happening in our team.
5. Seek improvements (for yourself, and for the team)
   • No matter how good you are, there are always more things you can learn, or skills you can get (note that knowledge and skills are different things..)
   • Improving and learning is your responsibility.
   • Not trying to understanding how things work, but learning that this does this, and that does that will lead you nowhere! Really! Even if you program for 20 years. I've seen such people.
6. Seek the most appropriate solution!.
   • There is no perfect code. There is no perfect solution also (or maybe there is, but it is not reachable).
   • There are no general good design rules - a design is good if it works well, it is easy to use (like if it is library, the clients need not to write much), is simple, has good performance, is modifiable/extensible, testable, etc. Nor the number of design patterns used solves this, nor a code convention... nothing. You have to find a design that is good.
   • Hacking around just to make things work is usually a time bomb. It is not acceptable to do this, because saving 1 day this month, usually costs 7 days the next month.
   • What we should do, is seeking something between just make it work, and perfect, but more close to perfect than just works.
   • That is, seek a good solution.
7. Quality is not negotiable! - but scope and resources are
   • If something can not be done in acceptable quality, given the resources (time) and scope (functionality) it has, the we either reduce the scope, or increase the resources.
   • We are not allowed to reduce the quality!
8. Accept the challenge!
   • We are not doing the next Store Information System, the next Lawyer's web site, nor the next database module for the big company X
   • What we are doing, is something that only few companies in the world can do, and even they are not good enough.
   • There is no "How to make the next generation desktop publishing software"!

[edit] Discipline

1. You have to try to follow the process as much as possible.
   • Self discipline is needed.
   • If you don't have enough self discipline, then this team is not appropriate for you (you should go to a company which forces you to be disciplined by other means).
2. By default, our work-time for half-time working (all of us currently) is 11:00 to 15:00.
   • If you need to shift it, or not work some days, but work more other days, you need to warn the leader, or the appropriate person.
   • TODO: maybe specify that you can not shift more than the half of the working time?
3. By default you have to make 20 working hours per week.
   • This means 20 productive work hour.
   • Not 20 hours staying in the office.
   • You may stay at the office as much as you like, take breaks, study, play games, whatever, but you should make 20 productive hours (working on a task).
4. You may work from home, or from other locations, but unless you have approval from the leader (like when we had no electricity), at least 3/4 of your time you should work from the office.
5. Unless having approval from the leader, you may not work more than 8 hours per day.
   • Or at least you may not count more than 8 hours per day as productive time.
   • This means, that you need to work at least 3 days per week in order to complete your time.
6. You should be present in at least 2/3 of the team meetings.
   • For one month, we usually have 10 meetings, which means you may skip at most 3.
7. In case you can not be present in a meeting, you have to warn the leader before this, and have a representative (someone else in the team) who will represent you.

[edit] Other

1. SVN commits should have clear message!
   • no commits without message
   • if it is hard to state what the change is, then probably you are not working on one thing.
2. Integrate continuously.
   • Do not hold a source code very long (or prolong the task).
3. Time box to 150% the estimate.
   • If you see that you are about to exceed the estimated time, warn the leader immediately.
   • If you leader agrees, and it seems that the task can be completed with little more time, it is possible to work a bit more on it.
   • A task reaching 150% (or 200% if the leader agrees) its estimate should be dropped, even if it is 99.9% complete.
4. Do not commit bad code!
   • Generally, you should not commit before a review is passed!

[edit]

SCRUM

We are continuing to use SCRUM (see Sophie-JR-Process-Optimization#Scrum). What is new, is that:

• Becoming much more strict (and unfortunately increasing the process overhead)
• Trying to have much higher quality.
• Hoping that the lost of speed will be compensated by non lost of momentum (that we usually suffer from)
• The sprint backlogs will be google-docs (TODO: link).

o Maybe we can continue to use the wall, but i'm not sure, because it will probably get unsynchronized quickly.

• The four state of a story/task will be changed

o from [not started, in progress, to be verified, done] o to [analyzing, designing, implementing, done]

• Each passing between states requires a review.

We create the internal backlog which should track:

• Impediments backlog
• ?Documentation backlog?? (reading tracking?)
• ?Code reviewing?

[edit]

Task Phases

Each taken task in the current sprint should pass trough the following phases:

• analyzing - task is waiting to be taken, and someone to determine What should be done?

o transitions:

+ to designing when all the post-conditions are met + to analyzing when post-conditions are not met

• designing - The implementor is designing it, that is, tries to decide How it should be done?

o pre-conditions:

+ the post-conditions of analyzing

o work:

+ designing is not only getting an idea of how to do it, but a detailed concept, + for many tasks the design phase should take most of the time

o post-conditions - see below o transitions:

+ to implementing if the reviewers decide, that the task is designed well + to designing if the reviewers decide, that the task needs better design, or does not satisfy the requirements + to analyzing if the reviewers or implementors decide that the task can not be done in acceptable time or have no idea how to do it

• implementing - The implementor makes an attempt to complete the task.

o pre-conditions:

+ the post-conditions of designing

• post-conditions - see below

o transitions:

+ to done if the reviewers decide, that there are no any issues. + to done if

# the reviewers decide, that there are very minor issues which should be fixed later # the issues are reported as bugs (or something) # the leader agrees on this # *note: it is usually better to face the reality, that it is just not done.

+ to implementing if the reviewers decide it does not fully satisfies the requirements + to designing if the reviewers decide, that the initial design will not do the work, and needs to be fixed. + to analyzing if the reviewers decide, that the task can not be done in acceptable time or quality.

• done

o when the task is really done

• *Note that, failing a state (for example moving from implementing to analyzing because implementing failed) requires that the state of the product is reversed to the initial.

You may note that some bigger tasks can not be easily made with these phases. However this is a problem to the task. Big tasks should be decomposed to smaller so that they can fit. [edit]

Requirements

I really hope that we don't have to do the great refactoring 3 (refactoring should be continuous process).

[edit] All Code Tasks

Both implementor and reviewer must be very careful. Please check that you understand all of the above.

For each code task (no matter whether it is a library, external feature, bug fix...), should pass the following check list:

• code

o The code style is correct to our convention. o The code is easily readable. o There are no fake JavaDocs (JavaDocs without useful information). o The java-doc is complete enough.

+ For each element, the JavaDoc should describe clearly what is it (if this is not easy, then probably the design is bad)

o It causes 0 errors. o It causes 0 warnings. o The @SuppressWarnings annotation is used only if really needed.

+ I know 3 cases for it -

# unused, when something is used only by reflection # synthetic-access, when you want to touch something internal with inner class (although in many cases this may be avoided) # unchecked, when you are doing something complex with generic (note that this is also avoidable in many cases)

o No inaccurate, or unclear XXX and TODO tags o It should not break other code (ensured by their tests). o other suggestions?

• design

o It is documented

+ (not only in the code).

o It is simple and understandable. o It is easy to use.

+ (using it requires less work / small code)

o It is error proof (hard to use it in a wrong way)

+ The compiler prevents most errors.

# (it is better to have an abstract getKind() instead of forcing implementors to call setKind at construction) # (final may also help)

+ Most of other errors cause an exception

# (defensive programming helps here)

o When a library is used, it is used correctly. o There are no representation exposures. o There are no GOD (very long) methods. o There are no GOD (very long) classes. o The coupling is low. o No magic numbers (and strings) o No array members.

+ (unless there is really a reason)

o No public instance fields. o No public non final fields o It has high cohesion.

+ (see ​http://en.wikipedia.org/wiki/Cohesion_(computer_science) )

o It has low coupling.

+ (see ​http://en.wikipedia.org/wiki/Coupling_%28computer_science%29 )

o It is possible to state in one sentence what every class or member is/does. o Classes and members are named correctly. o Every class (or attribute) is self responsible and self complete. o No code duplication (copy paste). o It is easily testable.

+ (when not, testing helpers are provided to make it such).

o No other kind of smells.

• It has good package structure.
• It is logging correctly.
• It has adequate error handling.
• No premature optimization.
• No premature abstraction.

• tests

o It has automatic use case tests, before implementing.

+ (they are very good to demonstrate the design for a library)

o If it has demos, the demos are actually tests with main method.

+ (demos should be done at design time!)

o It has enough unit tests to ensure its correctness

+ (listed at design time, passing at implementation time)

o It has enough integration tests

+ (listed at design time, passing at implementation time)

o It has enough system tests

+ (listed at design time, passing at implementation time)

o It has written scenarios for manual testing

+ (when it is an external feature or a bug)

o The tests had code coverage

+ (good code coverage does not mean good tests, but bad code coverage means bad tests)

• other:

o Enough information should be provided in the backlog.

[edit] Bad code samples

Here are some bad code examples (far from complete...) I have collected. Please check it, and ask if you don't understand why something is bad. This is important!

if (source instanceof RtfBookResource) {

RtfBookResource toCopy = (RtfBookResource) source; copiedBookResource = toCopy.clone(); try {

copiedBookResource.setMetaInfo(toCopy.getMetaInfo());

} catch (Exception e) {

throw new RuntimeException(e);

} target.resources().add(copiedBookResource);

} if (source instanceof BufferedImageBookResource) {

BufferedImageBookResource toCopy = (BufferedImageBookResource) source; copiedBookResource = toCopy.clone(); try {

copiedBookResource.setMetaInfo(toCopy.getMetaInfo());

} catch (Exception e) {

throw new RuntimeException(e);

} target.resources().add(copiedBookResource);

} if (source instanceof MediaBookResource) {

MediaBookResource toCopy = (MediaBookResource) source; copiedBookResource = new MediaBookResource(toCopy.getName(), toCopy

.getMedia());

try {

copiedBookResource.setMetaInfo(toCopy.getMetaInfo());

} catch (Exception e) {

throw new RuntimeException(e);

} target.resources().add(copiedBookResource);

}

Dont use deprecated API:

if(currentBook.getContainerFile()!=null) {

lastModified=new Date(currentBook.getContainerFile().lastModified()).toString(); bookSize=currentBook.getContainerFile().length();

}

Flag, int, etc are bad names:

boolean flag = true;

can be

boolean success = true;

or

boolean result = true;

Lots of array expose or redundancy

public Color[] HIGHLIGHT_COLORS = {Color.BLUE, Color.CYAN, Color.GREEN, Color.MAGENTA, Color.ORANGE, Color.PINK, Color.RED, Color.YELLOW};

Lots of lazy multi loading... in gui...

Bad JavaDoc - it should be focused on what is it doing, not how, or the details.

Initializes XXX - is bad for lazy loading setter.

readerMode is a bad name for checkbox... Many people did not get the idea of the Logic...

public void actionPerformed(final ActionEvent arg0) {

final int currentPageIndex = currentPageIndex(); int pageToGo;

try {

pageToGo = Integer

.parseInt(arg0.getActionCommand());

} catch (final Exception e) {

incorrect input JOptionPane.showMessageDialog(

swingRepresentation(), "Invalid input. Enter a number between 0 and "

+ model().get().pages().size(),

"Error", JOptionPane.ERROR_MESSAGE);

BookView.this.currentPageField().get().setText(

""

+ model().get().currentPage().get()

.getIndex());

return;

}

int maxPage = model().get().pages().size()

• (isInReaderMode() ? 1 : 0);

if ((pageToGo < 0)

pageToGo > maxPage) {

out of bounds JOptionPane.showMessageDialog(

swingRepresentation(), "Invalid page number. Enter a number between 0 and "

+ maxPage, "Error",

JOptionPane.ERROR_MESSAGE);

pageToGo = currentPageIndex;

}

controller().get().toPage(pageToGo);

assert model().get().currentPage().get() != null; BookView.this.currentPageField().get().setText(

""

+ model().get().currentPage().get()

.getIndex());

}

});

Splitting a big initialization into methods solves nothing!

public FrameView(final PageView parent, final Frame<?> frame,

final Class<? extends FrameController> subController) {

super(parent, frame, subController);

swingRepresentation();

initSizeAndLocation();

this.frameHaloMenu = new FrameHaloMenu(this);

this.frameHaloMenu.lockTo(swingRepresentation(),

RelativePosition.TOP_LEFT, RelativePosition.BOTTOM_LEFT);

detachHaloMenu();

initialize(); addListeners();

getParent().swingRepresentation().setLayer(swingRepresentation(),

getModel().getZorder());

getParent().swingRepresentation().add(swingRepresentation());

Logger.getRootLogger().info("Frame view created.");

updateLayer();

}

massive use of linked lists with no reason:

List<T> res = new LinkedList<T>();

lots of instance things that should be static (like arrays of data). reader mode code was extreme....

Bad naming (and style)

public String setPropertiesText(Book currentBook)

Hack the model for the purpose of the view..

example - the zOrder + 1 hack

Some people should ask more... especially when they think something they are

going to do is going to get ugly.

Bad comments:

/

• @author Presley
• */ is not nice comment for a class

Non-static nested classes dont need reference to outer class..

they have it implicitely

public BooksButton(final Book book,

final OpenBooksPalette openBooksPalette) {

super(book.getTitle());

No sub statements without {}

if (getTab().getFlap().getAppView().currentBookView().get() == null)

getTab().getFlap().getAppView().userNewBook();

Assign repeated expressions to locals variables

if (getTab().getFlap().getAppView().currentBookView().get() == null)

getTab().getFlap().getAppView().userNewBook();

int index = getList().locationToIndex(e.getPoint()); switch (index) { case 0:

getTab().getFlap().getAppView().getCurrentChild()

.getCurrentChild().userDropFrame(x, y,

FrameKind.Text);

break;

case 1:

getTab().getFlap().getAppView().getCurrentChild()

.getCurrentChild().userDropFrame(x, y,

FrameKind.Image);

break;

case 2:

getTab().getFlap().getAppView().getCurrentChild()

.getCurrentChild().userDropFrame(x, y,

FrameKind.Media);

break;

default:

ErrorHandlingUtilities.showErrorMessageDialog(

getList(), "Error while trying to create frame.", "Frames Palette Error");

break;

}

Sequence of ifs, or switch is a bad thing. It is strange that

someone created class ComponentsPaletteItem, but instead moving the functionallity there, had things like:

case 0:

curPage.userDropFrame(x, y, FrameKind.Text); break;

case 1:

curPage.userDropFrame(x, y, FrameKind.Image); break;

case 2:

curPage.userDropFrame(x, y, FrameKind.Media); break;

Do not inherit, unless really necessary. Composition is usually better...

public class ComponentsPaletteItem implements Transferable

ListPalette was refactored... for good. ListPaletteItem finally introduced

Search is refactored.

BookTemplates are refactored.

If you need the old code -- 2423 is the last stable revision.

create methods are supposed to create something (and return it)

private void createList(){

list().get().setSelectionMode(ListSelectionModel.SINGLE_SELECTION); createModel(); list().get().setCellRenderer(new ImageRenderer()); list().get().setDragEnabled(true); list().get().setTransferHandler(new PageTemplatesTransferHandler());

}

-> initList is better name

All menus were refactored... Now they are in the MainMenuBar class instead

the AppView.

FileChoser instance moved to AppView

ImageFrameView - we need to discuss the strange painting

This final everywhere is not needed. It is ok (and good) for

fields, but for local variable and arguments (which are not used in inner classes) it is too much.

Avoid code duplication... it leads to out of date copies:

} catch (Exception e) {

incorrect input JOptionPane.showMessageDialog(bookView.swingFrame().get(),

"Invalid input. Enter a number between 1 and "

+ bookView.model().get().pages().size(),

"Error", JOptionPane.ERROR_MESSAGE);

bookView.navigationPanel().get().currentPageField().get().setText("" + currentPageIndex); return;

}

int maxPage = bookView().get().model().get().pages().size()

• (bookView().get().inReaderMode().get() ? 1 : 0);

if ((pageToGo < 0)

pageToGo > maxPage) {

out of bounds JOptionPane.showMessageDialog(bookView().get().swingFrame().get(),

"Invalid page number. Enter a number between 0 and "

+ maxPage, "Error", JOptionPane.ERROR_MESSAGE);

pageToGo = currentPageIndex;

}

Don not prefix the static members related to the class... they are already scoped:

bad (class DecreaseBorderButton):

private static final String DECREASE_BORDER_TOOL_TIP = "Decrease the border of this frame.";

better (class DecreaseBorderButton):

private static final String TOOL_TIP = "Decrease the border of this frame.";

General

The following are valid for no matter what kind of task we are doing:

• Analysis - What is to be done?
• Design - How will we do it?
• Implementation - The actual "do it".

Review may be applied after each phase. Each activity should be logged (in the sprint backlog), and all information needed to log it should be filled. If implementors want, they can (and are encouraged to) ask for guidelines early. This is something like an early review, which is not logged.

At first month, each review should be rechecked by me (milo).

However, to clarify the things further:

• Implementing a new external feature (visible from the user).

o analyzing -

+ should have a specification, and a manual testing scenario

o designing -

+ write an automatic test/tests which verifies that your feature is working + look at the related code, + decide what needs to be added + if you add new library feature

# write use case tests for it

+ You may also write skeleton types (only declaration) and demos. + write an outline of your design.

o implementing

+ Make all the designed things work. + Ensure that all tests pass. + During the process, add more tests.

o reviewers - one other developer and one QA.

• Researching about a technology, library, whatever.

o analyzing - specify what needs to be researched

+ what the research will solve, + what is needed to be solved (this is important)

o designing

+ You can try things, etc, but... do not pollute the main source (for example with libraries). + If you need to use other libraries, do it in another project (or in another branch) + If you don't need other libraries, you can do it in a research package, but you should make sure that you don't introduce warnings, failing tests, etc.

o implementing

+ You present the written results / conclusions of your research, demo codes, etc.

o reviewers - 1-2 developers

• Implementing a new internal feature (a library or something, not directly visible).

o analyzing - what the library will provide o designing - should include

+ use case tests, + samples, + (in some cases demo), + design outline

o implementing

+ the library should be implemented + enough tests should be written during the process

o reviewers: 2 developers

• Performing some structure changes, or refactoring.

o analyzing - what the issues are o designing - understanding it, and providing an idea how to fix the issues o implementing - fixing the issues o reviewers: 1-2 developers

• Writing a specification

o analyzing - what needs to be specified o designing - brief ideas about the thing o implementing - writing the specification exactly o reviewers: - 1 qa + 1 developer

• Doing documentation.

o analyzing - what needs to be document o designing - what it will contain (outline) and understanding of the content o implementing - adding the appropriate content. o reviewers: - 1-2 team members

• External testing (the usual testing task).

o analyzing - has to state what the focus of the tests will be. o designing - has to select test scenarios, and make new if necessary o implementing - should apply testing scenarios and provide bugs and recommendations (things that probably the users will like/dislike) o reviewers: 1 developer

• Fixing a bug.

o analyzing - what the bug is. needs a manual testing scenario (in bugzilla) o designing - an automatic test (failing) should be present, idea what is causing it, more tests (which detect related internal bugs), idea how can be fixed o implementing - fixing the bug, making the tests pass, and some refactoring o reviewers: 1 QA and 1 developer

• Something else?

Templates for logging specific task kinds should be added.

Roles

• leader - someone assigned to manage something or (default: milo)
• implementors - someone or someones working on given task
• reviewers - someones (it needs to be two, usually one developer and one QA) which is assigned to review something.

o the reviewers and the implementors may not intersect!

Things to be read tracking - TODO

Other notes

Reverting

If some bad code is committed (for example one that breaks changes) there may be a need to revert it.

Reverting is done by:

• creating an inverting diff (between revisions, or several revision numbers)
• applying that diff to the current source code.
• commiting

TODO: the next that to it, please write the specific commands for this