kicad-developers team mailing list archive

[Vladimir Uryvaev <vovanius@xxxxx>]

Jean,


At Monday 21 of November 2011 15:27:59 from jp.charras:
> My first mail seem lost.
> Sorry if it is duplicated.

Yes, it seem to be lost.

> > This is possibly true, but you know, I have no right to miss single place
> > where intrusion is need, so the only way to be is to move through the
> > code step by step, revising line by line.
> 
> Are you sure ?
> Many time we must choose between drawbacks, not between advantages.
> Using intrusive code to track a possible bug (i.e. mainly overflow) is
> certainly good for new code. But for an existing code, this way has a
> hight cost:
> A *lot* of changes are needed, and many changes made twice:
>   once to add new code.
>   once to simplify this new code or remove it in many places.
> The cost is very hight:
>   It takes a long time and a lot of work.
>   It creates new bugs (typos... when adding and when simplifying the code).
> 
> I am not sure the benefit is hight ( I am pretty sure it is low). Here are
> my reasons: - the current code is well known and debugged. There are very
> few places where overflows can happen: - 2 or 3 functions using segment
> slope (I already fixed one)
>     - Some polygon calculations ( but for this case, the new code cannot
> trap overflow, I think) - And (but this is not an overflow issue) we can
> have issue with very small scaling factors in draw functions (mainly under
> Windows).
> So my concern is: new intrusive code will create
> (because the numerous changes in code, and typo errors created by these
> changes) more bugs than you can catch.
> 
> So here is my opinion ( 1 and 2 are very important ):
> 1 - accept the risk to find an unexpected overflow inside Kicad code (this
> is in fact a low risk). 2 - do not accept the risk to create some bugs due
> to typo errors and new complexity code (this is in fact a hight risk). 3 -
> do not spent time to modify existing code, outside Read/Write functions
> and few other functions. 4 - take in account there is already a support
> inside Kicad to handle conversion from/to internal units and strings in
> dialogs, or plotter units that never use decimils. 5 - Issues can be found
> outside Kicad code and will not trapped by intrusive code.

Your thoughts are so reasonable to just ignore them.

I've had some to meditate on a problem, so I've found some ideas. Altough the 
metod with 'cathing' class remains, there are some essential changes, and 
that's the new plan:

1. Before, I avoided uncontrolled implicit conversions between ints and 
lengths, which are object to trace. Now, I found, that at this stage I could 
write cast methods (LENGTH(int) and operator int()), which automatically call 
comversion between new and old units. So, while code wasn't completely 
checked, old and new units can live together, without explicit conversion.

2. Make VECTOR_PCB type direct replacement to wxPoint, so conversion to 
wxPoint should be just like with int.

3. Then come 'revert' will be done, so code would look as before, without 
FROM/TO_LEGACY_LU, and should compile and run correctly, and that's the proof 
that class LENGTH designed correctly.

3. And after all there only need to rewrite some 'problematic' code, e. g. 
dealing with references (int&), and add explicit conversions where they're 
need principally (mainly IO, GUI).

4. Then conversion is seem to be done, there should be no explicit conversions 
anymore, so if I switch of them, code should still compile and work correctly 
(if it doesn't, it's just need to iterate through step 3).

So after all LENGTH_PCB type can obviously be switched to int, and again, all 
should work, and then work is done.

While in process, the advantages of LENGTH class should remain, so wrong 
conversions and overflows are catchable.

I also think all the length framework can be kept after switched off, so 
developer if he suspect overflow and/or dimension problems may turn it on in 
debug process to dispel his doubts.

> Please, note also issues (not bugs) can be found in dialogs that display
> values: for instance: 1 mm is displayed as 1.0 mm
> but in inches:
>   0.0393700784 inch.
> Is 0.0393700784 pleasant in a dialog ?
> But if less digit are displayed (said 0.03937, more pleasant),
> what happen when the internal unit will be created from the dialog?

I've found an issue on it that have to be resolved. If that number is 
shrinked, after Ok is pushed, the rounded value will be saved and thus 
unintentionally changed. So there is need to keep it with precition enough to 
represent a 'quantum', which is approx to 0,00000004 mil, so at least 8 digits 
after the delimiter should stay, if it represented in mils.

I think that such count of digits is more pleasant than unexpected change in 
value. Altough I do not think, that there would be serious work in both unit 
sets simultaneously.

There may be another solution, which I like more: unit appear in edit field 
along the numeric value and thus editable. When length is to be displayed, 
conversion routine find best unit (keeping in mind user preference) and convert 
a value using it.

There may be following algorithm:
- If a value is not a multiple of 127 quantums, re representation in imperial 
units could be awful (like your example above), so the only choice is metric.
- Otherwise user preference is used.

> >> Other changes (but this is also the case in this new code) are related
> >> to some constants in constructors and some dialogs.
> >> 
> >> Note in dialogs, values are always converted to string using a scale
> >> factor depending on the internal units see PutValueInLocalUnits ,
> >> ReturnValueFromString functions and m_InternalUnits member of
> >> EDA_DRAW_FRAME).
> > 
> > Currently to mantain original version in working state, I wrote second
> > pair of conversion functions, only one of them could remain at the end.
> > 
> > BTW lets give some critic: names of that functions give no idea that they
> > are pair of inverses to each other, so I proposed another pair.
> 
> Better names make better code, at low cost.

Better names are open for discussion. Right?

I would like to propose some interfaces with naming schemes to do some rework 
and extend current code, but later, now I'm busy with nanometres.

-- 

--- KeyFP: DC07 D4C3 BB33 E596 CF5E  CEF9 D4E3 B618 65BA 2B61