2
\$\begingroup\$

After I've now officially announced, that I'm discontinuing my color picker for Windows, I'm about to embark on a journey through Linux development.


I was quite strong in Delphi on Windows, and see no valid reason to focus - not that I'm not trying to, in quite many languages - mainly on development in Lazarus on Linux, as of now in major version 2.0. Without wanting to pay for an IDE, I just decided to stick with Lazarus.


That's enough for starters. Let's get to my question.

I haven't developed in Delphi since version XE6 dated 2014, so I need to refresh my knowledge, and this question is about the very core of the new color picker for Linux. Feel free to add any answer or comment. If I did the smallest error, I need to know. The first class I've written is meant for keeping track of X, Y coordinates (of the mouse cursor), I generalized it.


unit Coords;

{$mode objfpc}{$H+}

interface

uses
    Classes;

type
    // Flexible X, Y coordinates object class.
    TCoords = class(TObject)

    // these declarations are accessible within this unit only
    private
        // this is the variable we are working with
        FCoords: TPoint;
        // property for this function is unnecessary, but I like it as it is
        function IsInitialized: Boolean;

    // these declarations are visible and accessible to all
    public
        // this creates instance of TCoords initialized to PointOutOfReach
        constructor Create; reintroduce; overload;
        // this creates instance of TCoords initialized to user given point
        constructor Create(const ACoords: TPoint); overload;
        // this creates instance of TCoords initialized to user given coordinates
        constructor Create(const AX, AY: Integer); overload;
        // this indicates if instance was initialized or not by the user
        property Initialized: Boolean read IsInitialized;
        // this works directly with private FCoords variable storing coordinates
        property P: TPoint read FCoords write FCoords;
        // these two are shortcuts for X, Y coordinates' direct access
        property X: Integer read FCoords.X write FCoords.X;
        property Y: Integer read FCoords.Y write FCoords.Y;

    end;

implementation

var
  // this gets initialized when loading this unit
  PointOutOfReach: TPoint;

constructor TCoords.Create;
begin
    inherited Create;
    // since called without argument, we have to ensure,
    // there are some corner-case coordinates, so that we can
    // differentiate between a [0:0] and uninitialized state
    FCoords := PointOutOfReach;
end;

constructor TCoords.Create(const ACoords: TPoint);
begin
    inherited Create;
    FCoords := ACoords;
end;

constructor TCoords.Create(const AX, AY: Integer);
begin
    inherited Create;
    FCoords := Point(AX, AY);
end;

function TCoords.IsInitialized: Boolean;
begin
    // this returns True in case FCoords has been initialized
    // initialized means here for the FCoords point to be different from PointOutOfReach
    // achieved either by calling `Create(APoint)`, or later overwriting PointOutOfReach
    Result := FCoords <> PointOutOfReach;
end;

initialization

    // initialize PointOutOfReach to "impossible" coordinates when loading unit
    PointOutOfReach := Point(MAXINT, MAXINT);

end.

Edit

Since there is not yet a review, I could edit, that I've added one useful function:

declaration

// this converts the coordinates to string (default delimiter set to colon)
function ToString(const Delimiter: string = ':'): string; reintroduce;

definition

function TCoords.ToString(const Delimiter: string = ':'): string;
begin
    // setting empty result for debugging purposes solely
    Result := '';
    // this can happen only if constructor TCoords.Create; has been used
    if not IsInitialized then
    begin
        raise Exception.Create('TCoords.ToString: `FCoords: TPoint` member has not yet been initialized');
    end;
    // it does not make sence for the user to input empty delimiter
    // as that would lead to concatenating XY without any delimiter
    if Delimiter.IsEmpty then
    begin
        raise Exception.Create('TCoords.ToString: `Delimiter: string` argument is empty');
    end;
    // example: X=0, Y=1, Delimiter=' x ' would return '0 x 1'
    Result := FCoords.X.ToString + Delimiter + FCoords.Y.ToString;
end;
\$\endgroup\$
1
\$\begingroup\$
unit Coords;
     ^

If you noticed the big letter, let me tell you this was a bad start. I saved the unit file as Coords.pas, which lead to a series of various files' edits. Not recommended. Stick to lower-case:

unit coords;

TCoords = class(TObject)

TObject class is implicit, so it may be omitted:

TCoords = class

private

This protects members on unit scope only. Since I intended to protect members on class scope, I now use:

strict private

function IsInitialized: Boolean;

This is not only unnecessary but it also clouded my judgment as for not having any getters nor setters, I now have:

function GetCoordX: Integer;
function GetCoordY: Integer;
function GetPoint: TPoint;
procedure SetCoordX(const NewX: Integer);
procedure SetCoordY(const NewY: Integer);
procedure SetPoint(const NewPoint: TPoint);

Definitions can be found at the bottom of this answer.


// ...

Excessive commenting I found not helpful, only making a mess.


I was not properly checking the input in all cases. This was remedied (I hope). Also, raising exceptions in case of error proved helpful.


The initialization section along with the PointOutOfReach variable can be deleted, the more code I have, the less of its usefulness I saw, using structures like this one has proven more helpful:

if (ACoords.X = MAXINT) or (ACoords.Y = MAXINT) then
begin
    raise Exception.Create('TCoords.Create: You cannot initialize `FCoords: TPoint` member to MAXINT coordinates');
end;

Code after review

unit coords;

{$mode objfpc}{$H+}

interface

uses
    Classes, SysUtils;

type
    TCoords = class
    strict private
        FCoords: TPoint;
        function GetCoordX: Integer;
        function GetCoordY: Integer;
        function GetPoint: TPoint;
        procedure SetCoordX(const NewX: Integer);
        procedure SetCoordY(const NewY: Integer);
        procedure SetPoint(const NewPoint: TPoint);
    public
        constructor Create; reintroduce; overload;
        constructor Create(const ACoords: TPoint); overload;
        constructor Create(const ACoordX, ACoordY: Integer); overload;
        function ToString(const Delimiter: string = ':'): string; reintroduce;
        property X: Integer read GetCoordX write SetCoordX;
        property Y: Integer read GetCoordY write SetCoordY;
        property P: TPoint read GetPoint write SetPoint;
    end;

implementation

constructor TCoords.Create;
begin
    inherited Create;

    FCoords := Point(MAXINT, MAXINT);
end;

constructor TCoords.Create(const ACoords: TPoint);
begin
    inherited Create;

    if (ACoords.X = MAXINT) or (ACoords.Y = MAXINT) then
    begin
        raise Exception.Create('TCoords.Create: You cannot initialize `FCoords: TPoint` member to MAXINT coordinates');
    end;

    FCoords := ACoords;
end;

constructor TCoords.Create(const ACoordX, ACoordY: Integer);
begin
    inherited Create;

    if (ACoordX = MAXINT) or (ACoordY = MAXINT) then
    begin
        raise Exception.Create('TCoords.Create: You cannot initialize `FCoords: TPoint` member to MAXINT coordinates');
    end;

    FCoords := Point(ACoordX, ACoordY);
end;

function TCoords.GetCoordX: Integer;
begin
    if FCoords.X = MAXINT then
    begin
        raise Exception.Create('TCoords.GetCoordX: `FCoords.X: Integer` is equal to MAXINT. It has not been initialized yet');
    end;

    Result := FCoords.X;
end;

function TCoords.GetCoordY: Integer;
begin
    if FCoords.Y = MAXINT then
    begin
        raise Exception.Create('TCoords.GetCoordY: `FCoords.Y: Integer` is equal to MAXINT. It has not been initialized yet');
    end;

    Result := FCoords.Y;
end;

procedure TCoords.SetCoordX(const NewX: Integer);
begin
    if NewX = MAXINT then
    begin
        raise Exception.Create('TCoords.SetCoordX: `NewX: Integer` value cannot equal MAXINT');
    end;

    FCoords.X := NewX;
end;

procedure TCoords.SetCoordY(const NewY: Integer);
begin
    if NewY = MAXINT then
    begin
        raise Exception.Create('TCoords.SetCoordY: `NewY: Integer` value cannot equal MAXINT');
    end;

    FCoords.Y := NewY;
end;

function TCoords.GetPoint: TPoint;
begin
    Result := Point(GetCoordX, GetCoordY);
end;

procedure TCoords.SetPoint(const NewPoint: TPoint);
begin
    SetCoordX(NewPoint.X);
    SetCoordY(NewPoint.Y);
end;

function TCoords.ToString(const Delimiter: string = ':'): string;
begin
    if Delimiter.IsEmpty then
    begin
        raise Exception.Create('TCoords.ToString: `Delimiter: string` argument is empty');
    end;

    // example: X=0, Y=1, Delimiter=' x ' would return '0 x 1'
    Result := GetCoordX.ToString + Delimiter + GetCoordY.ToString;
end;

end.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.