Re: [fpc-pascal] Compiler Warning: Constructor should be public

2009-11-11 Thread Graeme Geldenhuys
Flávio Etrusco wrote:
 
 Graeme, I guess the OP didn't want to reintroduce the 'Create'
 constructor with lower visibility, just implement a second constructor
 with private visiblity.
 It can be useful when implementing singletons and factories to avoid
 some types of misuse by an unattentive coworker...

The problem is that a constructor (in this case Create) in still always
public. So that unattentive coworker can very easily use the default
public Create() constructor and by-pass all your custom code in your
private constructor.

So that pretty much makes your custom private constructor useless. Plus
if the class was instantiated with the default public Create(), there
could be many other side-effects due to the fact that it by-passed your
custom code.

And yes I know, such a feature would be very handy for Singleton
implementations, but alternative solutions are possible. The same could
be applied to the original poster's code, without creating private or
protected constructors.

A very quick and dirty solution would be to reimplement the public
Create() constructor and immediately raise an exception inside it,
explaining the problem. That way if any developer tries to use that
default constructor, they will get an error - instead of some silent
side-effect later on. Then implement a new public custom constructor
with your desired code.

eg:

  TMyClass = class(TObject)
  public
constructor Create;
constructor CreateCustom(param1: string);
  end;

...

  constructor TMyClass.Create;
  begin
raise Exception.Create('Please use CreateCustom instead');
  end;

  constructor TMyClass.CreateCustom(param1: string);
  begin
inherited Create; // - note inherited call to bypass Exception
FValue := param1;

  end;


No private or protected constructors are used and you are guiding the
developer to only use the CreatCustom constructor, ensuring your custom
code is executed.

NOTE:
This is still not a perfect solution, but minimized the potential problems.


 IIRC the last time this issue was raised you were on the side for
 removing the warning - my opinion too ;-)

Maybe I learnt from my mistakes. ;-)


Regards,
  - Graeme -

-- 
fpGUI Toolkit - a cross-platform GUI toolkit using Free Pascal
http://opensoft.homeip.net/fpgui/

___
fpc-pascal maillist  -  fpc-pascal@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-pascal


Re: [fpc-pascal] Compiler Warning: Constructor should be public

2009-11-11 Thread fpclist
On Wednesday 11 November 2009 09:22:30 Graeme Geldenhuys wrote:
 Flávio Etrusco wrote:
  Graeme, I guess the OP didn't want to reintroduce the 'Create'
  constructor with lower visibility, just implement a second constructor
  with private visiblity.
  It can be useful when implementing singletons and factories to avoid
  some types of misuse by an unattentive coworker...

 The problem is that a constructor (in this case Create) in still always
 public. So that unattentive coworker can very easily use the default
 public Create() constructor and by-pass all your custom code in your
 private constructor.

Just a thought:

In FP, as with Delphi, TObject is the ultimate ancestor. Would it be possible 
to add a modified version of TObject to the RTL where a default constructor 
is not defined?

I know that TObject has a lot of functionality but would it be possible for a 
user-defined class to be defined without having to inherit from anything? A 
sort of user-defined TObject?

:=Nino
 

 So that pretty much makes your custom private constructor useless. Plus
 if the class was instantiated with the default public Create(), there
 could be many other side-effects due to the fact that it by-passed your
 custom code.

 And yes I know, such a feature would be very handy for Singleton
 implementations, but alternative solutions are possible. The same could
 be applied to the original poster's code, without creating private or
 protected constructors.

 A very quick and dirty solution would be to reimplement the public
 Create() constructor and immediately raise an exception inside it,
 explaining the problem. That way if any developer tries to use that
 default constructor, they will get an error - instead of some silent
 side-effect later on. Then implement a new public custom constructor
 with your desired code.

 eg:

   TMyClass = class(TObject)
   public
 constructor Create;
 constructor CreateCustom(param1: string);
   end;

 ...

   constructor TMyClass.Create;
   begin
 raise Exception.Create('Please use CreateCustom instead');
   end;

   constructor TMyClass.CreateCustom(param1: string);
   begin
 inherited Create; // - note inherited call to bypass Exception
 FValue := param1;
 
   end;


 No private or protected constructors are used and you are guiding the
 developer to only use the CreatCustom constructor, ensuring your custom
 code is executed.

 NOTE:
 This is still not a perfect solution, but minimized the potential problems.

  IIRC the last time this issue was raised you were on the side for
  removing the warning - my opinion too ;-)

 Maybe I learnt from my mistakes. ;-)


 Regards,
   - Graeme -



___
fpc-pascal maillist  -  fpc-pascal@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-pascal


Re: [fpc-pascal] Compiler Warning: Constructor should be public

2009-11-11 Thread Anthony Walter
Of course you can't hide methods previously exposed in an inherited
class. But this applies to every member type (fields, properties,
methods), not just constructors. The point of a constructor is not to
new up a class (NewInstance does that), but to execute a block of code
(with checks) before that new instance is returned. By allowing
developers to hide some constructors (private constructor
CreateFromLine for example), other developers are denied the access to
that block of code.

On Wed, Nov 11, 2009 at 2:22 AM, Graeme Geldenhuys
gra...@mastermaths.co.za wrote:
 Flávio Etrusco wrote:

 Graeme, I guess the OP didn't want to reintroduce the 'Create'
 constructor with lower visibility, just implement a second constructor
 with private visiblity.
 It can be useful when implementing singletons and factories to avoid
 some types of misuse by an unattentive coworker...

 The problem is that a constructor (in this case Create) in still always
 public. So that unattentive coworker can very easily use the default
 public Create() constructor and by-pass all your custom code in your
 private constructor.

 So that pretty much makes your custom private constructor useless. Plus
 if the class was instantiated with the default public Create(), there
 could be many other side-effects due to the fact that it by-passed your
 custom code.

 And yes I know, such a feature would be very handy for Singleton
 implementations, but alternative solutions are possible. The same could
 be applied to the original poster's code, without creating private or
 protected constructors.

 A very quick and dirty solution would be to reimplement the public
 Create() constructor and immediately raise an exception inside it,
 explaining the problem. That way if any developer tries to use that
 default constructor, they will get an error - instead of some silent
 side-effect later on. Then implement a new public custom constructor
 with your desired code.

 eg:

  TMyClass = class(TObject)
  public
    constructor Create;
    constructor CreateCustom(param1: string);
  end;

 ...

  constructor TMyClass.Create;
  begin
    raise Exception.Create('Please use CreateCustom instead');
  end;

  constructor TMyClass.CreateCustom(param1: string);
  begin
    inherited Create; // - note inherited call to bypass Exception
    FValue := param1;
    
  end;


 No private or protected constructors are used and you are guiding the
 developer to only use the CreatCustom constructor, ensuring your custom
 code is executed.

 NOTE:
 This is still not a perfect solution, but minimized the potential problems.


 IIRC the last time this issue was raised you were on the side for
 removing the warning - my opinion too ;-)

 Maybe I learnt from my mistakes. ;-)


 Regards,
  - Graeme -

 --
 fpGUI Toolkit - a cross-platform GUI toolkit using Free Pascal
 http://opensoft.homeip.net/fpgui/

 ___
 fpc-pascal maillist  -  fpc-pas...@lists.freepascal.org
 http://lists.freepascal.org/mailman/listinfo/fpc-pascal

___
fpc-pascal maillist  -  fpc-pascal@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-pascal


Re: [fpc-pascal] Compiler Warning: Constructor should be public

2009-11-11 Thread Vinzent Höfler
Graeme Geldenhuys gra...@mastermaths.co.za:

 A very quick and dirty solution would be to reimplement the public
 Create() constructor and immediately raise an exception inside it,
 explaining the problem. That way if any developer tries to use that
 default constructor, they will get an error - instead of some silent
 side-effect later on. Then implement a new public custom constructor
 with your desired code.

Why public then? Perhaps I don't *want* the user to create instances of this 
class in an uncontrolled way?


Vinzent.
-- 
Jetzt kostenlos herunterladen: Internet Explorer 8 und Mozilla Firefox 3.5 -
sicherer, schneller und einfacher! http://portal.gmx.net/de/go/chbrowser
___
fpc-pascal maillist  -  fpc-pascal@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-pascal


Re: [fpc-pascal] Compiler Warning: Constructor should be public

2009-11-10 Thread Jonas Maebe


On 10 Nov 2009, at 14:35, Anthony Walter wrote:


In my opinion the warning should be removed, or at least able to be
suppress through a switch.


FPC 2.4.0 will include the ability to suppress individual messages.  
Use -vq to show the messages numbers, and -vmxxx to suppress a  
particular message. You can download 2.4.0 release candidate 1 from  
the link on our website's home page (the call for testing was sent to  
the fpc-devel mailing list).



Jonas
___
fpc-pascal maillist  -  fpc-pascal@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-pascal


Re: [fpc-pascal] Compiler Warning: Constructor should be public

2009-11-10 Thread Graeme Geldenhuys
Anthony Walter wrote:
 
 In my opinion the warning should be removed, or at least able to be
 suppress through a switch. I beginning to make the transition to cross

The compiler is 100% and I think it should actually raise an Error
instead of a Warning.

Your code is flawed. Object Pascal is quite clear regarding visibility
rules. TObject has a Public constructor. You can only raise the
visibility from there, not reduce visibility.

Imagine how confusing it will be if you code was a library of some sorts
and other developers had to use it. In a inherited class, you have
public visibility to the constructor or some other method, and then
suddenly in the descendant class the methods are hidden from the
developer??  Not a good idea.

You can try out the code and see what I mean. Even in Delphi you cannot
reduce a methods visibility, only make it more visible.

In your code you posted, you cannot hide the constructor (one of them
will always be visible). And it will be like that for both Delphi and
FPC no matter if you try and hide the constructor or not. The default
TObject.Create constructor is *always* visible from Public onwards.


But like Jonas said, in newer compiler versions you can hide compiler
messages, but that doesn't change the flaw in your code.

  raise EAssertError.Create;

is perfectly legal, and will by-pass whatever you intended in your
CreateFromLine() constructor.


Regards,
  - Graeme -

-- 
fpGUI Toolkit - a cross-platform GUI toolkit using Free Pascal
http://opensoft.homeip.net/fpgui/

___
fpc-pascal maillist  -  fpc-pascal@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-pascal


Re: [fpc-pascal] Compiler Warning: Constructor should be public

2009-11-10 Thread Anthony Walter
Your argument is flawed. Visibility rules apply to identifiers, not keywords.

Create // identifier
CreateFromLine // different identifier

No visibility is being changed by introducing a *new* identifier

raise EAssertError.Create(Msg); // still visible
raise EAssertError.CreateFromLine(Msg, FileName, LineNumber); // not visible

On Tue, Nov 10, 2009 at 8:56 AM, Graeme Geldenhuys
gra...@mastermaths.co.za wrote:

 Anthony Walter wrote:
 
  In my opinion the warning should be removed, or at least able to be
  suppress through a switch. I beginning to make the transition to cross

 The compiler is 100% and I think it should actually raise an Error
 instead of a Warning.

 Your code is flawed. Object Pascal is quite clear regarding visibility
 rules. TObject has a Public constructor. You can only raise the
 visibility from there, not reduce visibility.
___
fpc-pascal maillist  -  fpc-pascal@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-pascal


Re: [fpc-pascal] Compiler Warning: Constructor should be public

2009-11-10 Thread Paul Nicholls
- Original Message - 
From: Anthony Walter sys...@gmail.com
To: grae...@opensoft.homeip.net; FPC-Pascal users discussions 
fpc-pascal@lists.freepascal.org

Sent: Wednesday, November 11, 2009 1:07 AM
Subject: Re: [fpc-pascal] Compiler Warning: Constructor should be public


Your argument is flawed. Visibility rules apply to identifiers, not 
keywords.


Create // identifier
CreateFromLine // different identifier

No visibility is being changed by introducing a *new* identifier

raise EAssertError.Create(Msg); // still visible
raise EAssertError.CreateFromLine(Msg, FileName, LineNumber); // not 
visible



SNIP

Hi Anthony,
   Sorry, but you are incorrect; Create is a method, not an identifier, 
and like all methods, it is affected by visibility rules (public, private, 
protected).


cheers,
Paul 


___
fpc-pascal maillist  -  fpc-pascal@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-pascal


Re: [fpc-pascal] Compiler Warning: Constructor should be public

2009-11-10 Thread Anthony Walter
Sorry, Create is an identifier.

constructor TSampleForm.Create(AOwner: TComponent);
var
  Create: Integer;
begin
  inherited;
  Create := Tag;
  Caption := IntToStr(Create);
end;

Aside from that, if you read you will see I used two identifiers ...
one called Create and another called ... get this CreateFromLine.
See that? It's a different identifier (it's not the same).

Test it out yourself.

if 'Create' = 'CreateFromLine' then ; // always evaluates to false

As such, the compiler shouldn't issue a warning about constructors
must be public.

protected constructor CreateFromData(var Data); // there should not be
a warning saying CreateFromData must be public

It should however issue a waring when you try to alter the visbility
of a method:

private procedure FreeInstance; override; // warning or error here is expected

On Tue, Nov 10, 2009 at 4:35 PM, Paul Nicholls paulfnicho...@gmail.com wrote:
 - Original Message - From: Anthony Walter sys...@gmail.com
 To: grae...@opensoft.homeip.net; FPC-Pascal users discussions
 fpc-pascal@lists.freepascal.org
 Sent: Wednesday, November 11, 2009 1:07 AM
 Subject: Re: [fpc-pascal] Compiler Warning: Constructor should be public


 Your argument is flawed. Visibility rules apply to identifiers, not
 keywords.

 Create // identifier
 CreateFromLine // different identifier

 No visibility is being changed by introducing a *new* identifier

 raise EAssertError.Create(Msg); // still visible
 raise EAssertError.CreateFromLine(Msg, FileName, LineNumber); // not
 visible

 SNIP

 Hi Anthony,
   Sorry, but you are incorrect; Create is a method, not an identifier, and
 like all methods, it is affected by visibility rules (public, private,
 protected).

 cheers,
 Paul
 ___
 fpc-pascal maillist  -  fpc-pas...@lists.freepascal.org
 http://lists.freepascal.org/mailman/listinfo/fpc-pascal

___
fpc-pascal maillist  -  fpc-pascal@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-pascal


Re: [fpc-pascal] Compiler Warning: Constructor should be public

2009-11-10 Thread Flávio Etrusco
On Tue, Nov 10, 2009 at 10:56 AM, Graeme Geldenhuys
gra...@mastermaths.co.za wrote:
 Anthony Walter wrote:

 In my opinion the warning should be removed, or at least able to be
 suppress through a switch. I beginning to make the transition to cross

 The compiler is 100% and I think it should actually raise an Error
 instead of a Warning.

 Your code is flawed. Object Pascal is quite clear regarding visibility
 rules. TObject has a Public constructor. You can only raise the
 visibility from there, not reduce visibility.

(...)

  raise EAssertError.Create;

 is perfectly legal, and will by-pass whatever you intended in your
 CreateFromLine() constructor.


Graeme, I guess the OP didn't want to reintroduce the 'Create'
constructor with lower visibility, just implement a second constructor
with private visiblity.
It can be useful when implementing singletons and factories to avoid
some types of misuse by an unattentive coworker...
IIRC the last time this issue was raised you were on the side for
removing the warning - my opinion too ;-)

-Flávio
___
fpc-pascal maillist  -  fpc-pascal@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-pascal


Re: [fpc-pascal] Compiler Warning: Constructor should be public

2009-11-10 Thread Anthony Walter
QFT, Flávio said:

Graeme, I guess the OP didn't want to reintroduce the 'Create'
constructor with lower visibility, just implement a second constructor
(** with a different name **) with private visiblity. It can be useful
when implementing singletons and factories to avoid some types of
misuse by an unattentive coworker...

IIRC the last time this issue was raised you were on the side for
removing the warning - my opinion too ;-)

Yes! And well put. *Thank you*

2009/11/10 Flávio Etrusco flavio.etru...@gmail.com:
 On Tue, Nov 10, 2009 at 10:56 AM, Graeme Geldenhuys
 gra...@mastermaths.co.za wrote:
 Anthony Walter wrote:

 In my opinion the warning should be removed, or at least able to be
 suppress through a switch. I beginning to make the transition to cross

 The compiler is 100% and I think it should actually raise an Error
 instead of a Warning.

 Your code is flawed. Object Pascal is quite clear regarding visibility
 rules. TObject has a Public constructor. You can only raise the
 visibility from there, not reduce visibility.

 (...)

  raise EAssertError.Create;

 is perfectly legal, and will by-pass whatever you intended in your
 CreateFromLine() constructor.


 Graeme, I guess the OP didn't want to reintroduce the 'Create'
 constructor with lower visibility, just implement a second constructor
 with private visiblity.
 It can be useful when implementing singletons and factories to avoid
 some types of misuse by an unattentive coworker...
 IIRC the last time this issue was raised you were on the side for
 removing the warning - my opinion too ;-)

 -Flávio
 ___
 fpc-pascal maillist  -  fpc-pas...@lists.freepascal.org
 http://lists.freepascal.org/mailman/listinfo/fpc-pascal

___
fpc-pascal maillist  -  fpc-pascal@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-pascal