View Issue Details

IDProjectCategoryView StatusLast Update
0005916JEDI VCL00 JVCL Componentspublic2013-12-13 11:18
ReporterdmgamerAssigned Toobones 
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionno change required 
Product VersionDaily / GIT 
Target VersionFixed in Version 
Summary0005916: Correction of 0005512 create new "bug"
DescriptionIm use JvCalcEdit for Number Edit Control and use Text property to access the value of component.
After 0005512 modification, when access value great then 3 digits (ex. 1234) value returned is "1.234" and error "is not a valid integer value" occurs.

EdtNumIni.Value := 1234;
qry.SQL.Text := 'where NumIni = ' + EdtNumIni.Text;

work around for me is:
  if FDecimalPlaces > 0 then
  begin
    Result := ',0'; // must put the thousands separator by default to allow
                        // direct edit of value (paste for example)
    if FDecimalPlacesAlwaysShown then
       Result := Result + '.' + MakeStr('0', FDecimalPlaces)
    else
       Result := Result + '.' + MakeStr('#', FDecimalPlaces);
  end
  else
    Result := '0';
TagsNo tags attached.

Relationships

related to 0005512 resolvedobones TJVDBCalcEdit Property DecimalPlaces AlwaysShown Incompatible with DefaultDisplayFormat 

Activities

Arioch

2012-06-27 13:09

developer   ~0020032

Last edited: 2012-06-27 13:09

qry.SQL.Text := 'where NumIni = ' + EdtNumIni.Text;

Isn't this line fragile on the verge on incorrectness ?

To me on should better use SQL parameters in such circumstances.
Or use some specific SQL-tuned IntToStr analogue.

After all there are .Value and .AsInteger computer-reading properties, and .Text is human-reading one, for display rather than for program's use

Personally i think that if Thousand Separator is specified, then it should be used for both float and integer values.

And if someone need to hide Thousand Separator, then there should be property for that. Though it seems like unneeded overriding of user-specified national settings

acgubamg

2012-06-28 04:34

reporter   ~0020033

why not let the programmer decide the property displaytext for example?

Arioch

2012-06-28 08:43

developer   ~0020034

It is quite not easy to sync such a generic property with all those "dumb knobs" like FDecimalPlacesAlwaysShown and FDecimalPlaces and such

you just toggle it out of curiosity or by accident or by legacy code - and your template is lost. Try to implement all that, having full two-way sync with all the rest Display-related properties, and proper interaction with system-default OS settings, that might change while program is running.

-------

I frankly think that even existing properties should better be hidden or removed ;-) i really think such tuning is bad practice. In Soviet school there was standard for comma as decimal separator and dot as thousand separator. US standard is dot and space respectively. As a compromise current common pattern is to have comma for decimals and space for thousands.

One place i worked at had an internal information system probably written in the manner like yours above. So, the 1st thing any new employee, allowed into that information system (not everyone was), had to do - open control panel and override national format settings to have no thousand separator and dot for decimal separator. If he forgot or did not know - then the system crashed with quite cryptic messages. So at home or at some related companies you had 3,14 and at work you had 3.14
That is small nuisance, but it covered all the working days.

---------

So really, i believe that u only should rely on textual representation as the last resort if nothing rest possible. If not for user comfort, then for security and reliability. And even performance.

Think about why do u use Delphi and not PHP, Visual Basic or some other dynamic untyped (or single-typed, i like that term ) language? Why do u use string and integer rather tan using Variant ? Think, why ?
If it has no sense for you, then maybe you'd really switch language to some typeless one ?
If it has sense to you - then do-not-do-it !

Use TDataSet.Params and .AsInteger instead. Use SQLQuery.Prepare, which would be possible with params.

Afterall, did you ever heard of "SQL Injections" ? PHP was started with code like that. But soon after came components, modeled after the same patter as TDataset with parameters. And... "no, it is too complex, too dull, too paranoid", PHPers still used string concatenation instead. Sometimes they added some ad-hoc syntax checks, that were incomplete and tended top be fragile and more complex than using params from beginning.

I don't remember if JVCE allows non-numeric input right away. But i can imagine some user might request it and get it. To allow number or free-form entry. Resulting in functionality kinda Combobox for example, choosing TextDescription[i] instead of plain number. Or like that.
Suddenly after upgrade ALL your used calcedits would be starting point for SQL Injection, and you would not know it!

dmgamer

2012-06-29 01:26

reporter   ~0020041

well ... understand the reasons and the suggestions ...
was not me who wrote the application, not usually use this way.
My problem is that the system is quite large and there are many screens to correct this "misuse" of the component ... and the previous version did not have this "problem" ...
Thank you for having responded ... (I'm screwed ^ ^))

Arioch

2012-06-29 07:54

developer   ~0020043

i feel ur pain.
at work i have to move to XE2 and refactor an app, that was developed for 15+ years since Delphi 2 without plan, without source control, without bug tracking, with half programmers being C++ guru, so program is split into DLLs and heavily use PChars... And the sources are about hundred MB. Okay, not merely sources, there compiled DLLs here and there. Code duplication all around. "Do not touch! it is working and no one knows how", etc

I really wish there were tools to make AST-aware search-and-replace.

or example i have dozens of places where there are
  StrLCopy(some-var, some-other-var, sizeof(some-var));
  some-var[sizeof(some-var)] := #0;

Okay, i even did macrorecord to turn it to proper
  StrLCopy(some-var, some-other-var, Length(some-var) - 1);

But that fails if some-var is not array. Or if last parameter is not sizof(var) but sizeof(type).

So, i feel your pain, yet... I don't know how to help you and myself :-)

Arioch

2012-06-29 07:59

developer   ~0020044

Maybe on your place i'd make my own component descendant from JvCalcEdit which would simulate all that. Or even such un ugly hack as to make you're own class with same name but different unit, descending from JVCL's and overriding Text property to be write-only. Or u can make it in .INC file.
Then u quickly ad that unit (or include) right before the form in all units, and all such code snippets suddenly no more compile, so at least u can hunt them down in more or less continuous session.

obones

2013-01-15 15:10

administrator   ~0020343

So, what's the status on this?

dmgamer

2013-01-15 15:55

reporter   ~0020371

For the design problem where he'm using a modified version so that the Text property does not return the thousands separator.
For new designs, do not use the syntax problematic.

Issue History

Date Modified Username Field Change
2012-06-26 00:00 dmgamer New Issue
2012-06-27 13:09 Arioch Note Added: 0020032
2012-06-27 13:09 Arioch Note Edited: 0020032
2012-06-27 13:10 Arioch Relationship added related to 0005512
2012-06-28 04:34 acgubamg Note Added: 0020033
2012-06-28 08:43 Arioch Note Added: 0020034
2012-06-29 01:26 dmgamer Note Added: 0020041
2012-06-29 07:54 Arioch Note Added: 0020043
2012-06-29 07:59 Arioch Note Added: 0020044
2013-01-15 15:10 obones Note Added: 0020343
2013-01-15 15:10 obones Status new => feedback
2013-01-15 15:55 dmgamer Note Added: 0020371
2013-12-13 11:18 obones Status feedback => resolved
2013-12-13 11:18 obones Resolution open => no change required
2013-12-13 11:18 obones Assigned To => obones