3
\$\begingroup\$

Intro

If you want to know, then about 10+ years ago, I started a journey on the (best in my country) color picker for WinXP, later Win7. Since now it is hardly compatible with Win10 and HiDPI (work in progress), and in no way compilable in Linux using Lazarus, it would take quite an effort to create a decent desktop color picker with zoom and everything on Linux. But first things first - in this question I would like to start with basic color class, i.e. class capable of holding single color, and providing easy access to each color component, except for hue sat lum, which I'm not able to program, and am not asking you to point me to any direction. At the bottom I attached this code to existing color picker for windows just to show what it is doing. I want to start from scratch and maybe re-invent the wheel just to practice my brain.


TBasicColor class

unit basic_color;
interface
uses
 Graphics;
type
 TBasicColor = class
 strict private
 FColorRef: TColor;
 protected
 function GetColorRef: Integer;
 function GetRed: Byte;
 function GetGreen: Byte;
 function GetBlue: Byte;
 function GetCyan: Byte;
 function GetMagenta: Byte;
 function GetYellow: Byte;
 public
 constructor Create; reintroduce;
 constructor CreateRandom;
 constructor CreateColorUsingRGB(const ARed, AGreen, ABlue: Byte);
 constructor CreateColorUsingCMY(const ACyan, AMagenta, AYellow: Byte);
 property ColorRef: Integer read GetColorRef;
 property Red: Byte read GetRed;
 property Green: Byte read GetGreen;
 property Blue: Byte read GetBlue;
 property Cyan: Byte read GetCyan;
 property Magenta: Byte read GetMagenta;
 property Yellow: Byte read GetYellow;
 end;
implementation
constructor TBasicColor.Create;
begin
 inherited Create;
 // implicitly initialize to white color
 CreateColorUsingRGB(255, 255, 255);
end;
constructor TBasicColor.CreateRandom;
begin
 inherited Create;
 FColorRef := Random($FFFFFF + 1);
end;
constructor TBasicColor.CreateColorUsingRGB(const ARed, AGreen, ABlue: Byte);
begin
 inherited Create;
 FColorRef := ARed or (AGreen shl 8) or (ABlue shl 16);
end;
constructor TBasicColor.CreateColorUsingCMY(const ACyan, AMagenta, AYellow: Byte);
begin
 CreateColorUsingRGB(255 - ACyan, 255 - AMagenta, 255 - AYellow);
end;
function TBasicColor.GetColorRef: Integer;
begin
 Result := Integer(FColorRef);
end;
function TBasicColor.GetRed: Byte;
begin
 Result := Byte(FColorRef);
end;
function TBasicColor.GetGreen: Byte;
begin
 Result := Byte(FColorRef shr 8);
end;
function TBasicColor.GetBlue: Byte;
begin
 Result := Byte(FColorRef shr 16);
end;
function TBasicColor.GetCyan: Byte;
begin
 Result := 255 - GetRed;
end;
function TBasicColor.GetMagenta: Byte;
begin
 Result := 255 - GetGreen;
end;
function TBasicColor.GetYellow: Byte;
begin
 Result := 255 - GetBlue;
end;
end.

Screenshot from Windows

Screenshot from Windows


Whatever you answer, I value all input. Thank you.

asked Aug 14, 2020 at 9:45
\$\endgroup\$
1
  • \$\begingroup\$ A bit late, but here it goes ;-) I wouldn't make this a class but an advanced record a.ka. a record with methods. \$\endgroup\$ Commented Jun 1, 2021 at 10:00

1 Answer 1

1
\$\begingroup\$

Self-review


Randomize missing

Since I use Random to generate pseudo-random color, it is, I lightly remember, a must to call Randomize once program or unit in this case is being created; more information.

// ...
initialization
 Randomize;
end.

Get... Red, Green, Blue, Cyan, Magenta, Yellow

These seem to operate as expected.


constructors

The biggest error I already see I have made is the constructor thing. The problem is I did not write one for direct TColor assignment. It could be re-written for instance like this in the interface:

type
 TBasicColor = class
 // ...
 public
 // default direct TColor assignment constructor
 constructor Create(const AColor: TColor); overload;
 // reintroduce is hiding TObject constructor
 constructor Create; reintroduce; overload;
 // create using RGB values
 constructor CreateRGB(const ARed, AGreen, ABlue: Byte);
 // create using CMY values
 constructor CreateCMY(const ACyan, AMagenta, AYellow: Byte);
 // create pseudo-random color constructor
 constructor CreateRandom;
 // ...

Plus, like this in the implementation:

constructor TBasicColor.Create(const AColor: TColor);
begin
 // in here it is just plain assignment
 inherited Create;
 FColorRef := AColor;
end;
constructor TBasicColor.Create;
begin
 // in case anyone just calls Create() we assign white color
 Create($FFFFFF);
end;
constructor TBasicColor.CreateRGB(const ARed, AGreen, ABlue: Byte);
begin
 Create(ARed or (AGreen shl 8) or (ABlue shl 16));
end;
constructor TBasicColor.CreateCMY(const ACyan, AMagenta, AYellow: Byte);
begin
 CreateRGB(255 - ACyan, 255 - AMagenta, 255 - AYellow);
end;
constructor TBasicColor.CreateRandom;
begin
 Create(Random($FFFFFF + 1));
end;

As you can see, all, in the end, are calling the default constructor, which I see as much better implementation.


overload keyword

Note on overload keyword, I originally did not need it in Lazarus, but Delphi requires it.


Comments

By the way, I should really use more comments, they will prove useful one day, once I read it after years.


Why read-only ColorRef?

On second thought, I see no reason for the ColorRef not being able to change at runtime, I find it hard to see what reason I had before, but no matter, it should stay a private member with properties to safely read and write, also the typecast might be wrong, cannot confirm or disprove at this point, best to typecast when necessary in-place.

For example with private method Assign:

procedure TBasicColor.Assign(const ColorRef: TColor);
begin
 if (ColorRef < 0) or (ColorRef > $FFFFFF)
 then raise ERangeError.Create('ERangeError in TBasicColor class.' + sLineBreak +
 'It supports only subset of TColor range.' + sLineBreak +
 'Valid range is <0; $FFFFFF>.')
 else FColorRef := ColorRef;
end;

which can in turn be used in the SetColorRef setter:

procedure TBasicColor.SetColorRef(const ColorRef: TColor);
begin
 Assign(ColorRef);
end;

ARed change to Red, etc.

I believe it's a habit or style point, but anyway.

I removed, and am no longer a fan of an A prefixing, changed to this:

constructor TBasicColor.CreateRGB(const Red, Green, Blue: Byte);
constructor TBasicColor.CreateCMY(const Cyan, Magenta, Yellow: Byte);

Modified code

After a few other adjustments, I will name only the use of setters in all color components, this unit could be re-written finally to this state:

unit basic_color;
interface
uses
 Graphics, SysUtils;
type
 TBasicColor = class
 strict private
 FColorRef: TColor;
 private
 // TColor assignment with range check <0; $FFFFFF>
 procedure Assign(const ColorRef: TColor);
 // independent function needed (Delphi/Lazarus; Windows/Linux)
 function RGBToColor(const Red, Green, Blue: Byte): TColor;
 protected
 function GetColorRef: TColor;
 procedure SetColorRef(const ColorRef: TColor);
 function GetRed: Byte;
 procedure SetRed(const NewRed: Byte);
 function GetGreen: Byte;
 procedure SetGreen(const NewGreen: Byte);
 function GetBlue: Byte;
 procedure SetBlue(const NewBlue: Byte);
 function GetCyan: Byte;
 procedure SetCyan(const NewCyan: Byte);
 function GetMagenta: Byte;
 procedure SetMagenta(const NewMagenta: Byte);
 function GetYellow: Byte;
 procedure SetYellow(const NewYellow: Byte);
 public
 // default direct TColor assignment
 constructor Create(const ColorRef: TColor); overload;
 // reintroduce is hiding TObject default constructor
 constructor Create; reintroduce; overload;
 // create color using RGB values
 constructor CreateRGB(const Red, Green, Blue: Byte);
 // create color using CMY values
 constructor CreateCMY(const Cyan, Magenta, Yellow: Byte);
 // create pseudo-random color
 constructor CreateRandom;
 property ColorRef: TColor read GetColorRef write SetColorRef;
 property Red: Byte read GetRed write SetRed;
 property Green: Byte read GetGreen write SetGreen;
 property Blue: Byte read GetBlue write SetBlue;
 property Cyan: Byte read GetCyan write SetCyan;
 property Magenta: Byte read GetMagenta write SetMagenta;
 property Yellow: Byte read GetYellow write SetYellow;
 end;
implementation
procedure TBasicColor.Assign(const ColorRef: TColor);
begin
 if (ColorRef < 0) or (ColorRef > $FFFFFF)
 then raise ERangeError.Create('ERangeError in TBasicColor class.' + sLineBreak +
 'It supports only subset of TColor range.' + sLineBreak +
 'Valid TBasicColor range is <0; $FFFFFF>.')
 else FColorRef := ColorRef;
end;
function TBasicColor.RGBToColor(const Red, Green, Blue: Byte): TColor;
begin
 Result := Red or (Green shl 8) or (Blue shl 16);
end;
constructor TBasicColor.Create(const ColorRef: TColor);
begin
 // in here it is just plain assignment
 inherited Create;
 Assign(ColorRef);
end;
constructor TBasicColor.Create;
begin
 // in case anyone just calls Create() we assign white color
 Create($FFFFFF);
end;
constructor TBasicColor.CreateRGB(const Red, Green, Blue: Byte);
begin
 Create(RGBToColor(Red, Green, Blue));
end;
constructor TBasicColor.CreateCMY(const Cyan, Magenta, Yellow: Byte);
begin
 CreateRGB(255 - Cyan, 255 - Magenta, 255 - Yellow);
end;
constructor TBasicColor.CreateRandom;
begin
 Create(Random($FFFFFF + 1));
end;
function TBasicColor.GetColorRef: TColor;
begin
 Result := FColorRef;
end;
procedure TBasicColor.SetColorRef(const ColorRef: TColor);
begin
 Assign(ColorRef);
end;
function TBasicColor.GetRed: Byte;
begin
 Result := Byte(FColorRef);
end;
procedure TBasicColor.SetRed(const NewRed: Byte);
begin
 Assign(RGBToColor(NewRed, GetGreen, GetBlue));
end;
function TBasicColor.GetGreen: Byte;
begin
 Result := Byte(FColorRef shr 8);
end;
procedure TBasicColor.SetGreen(const NewGreen: Byte);
begin
 Assign(RGBToColor(GetRed, NewGreen, GetBlue));
end;
function TBasicColor.GetBlue: Byte;
begin
 Result := Byte(FColorRef shr 16);
end;
procedure TBasicColor.SetBlue(const NewBlue: Byte);
begin
 Assign(RGBToColor(GetRed, GetGreen, NewBlue));
end;
function TBasicColor.GetCyan: Byte;
begin
 Result := 255 - GetRed;
end;
procedure TBasicColor.SetCyan(const NewCyan: Byte);
begin
 SetRed(255 - NewCyan);
end;
function TBasicColor.GetMagenta: Byte;
begin
 Result := 255 - GetGreen;
end;
procedure TBasicColor.SetMagenta(const NewMagenta: Byte);
begin
 SetGreen(255 - NewMagenta);
end;
function TBasicColor.GetYellow: Byte;
begin
 Result := 255 - GetBlue;
end;
procedure TBasicColor.SetYellow(const NewYellow: Byte);
begin
 SetBlue(255 - NewYellow);
end;
initialization
 Randomize;
end.
answered Aug 14, 2020 at 12:44
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.