I need feedback

I need feedback

[juanmiuk]

My name is Juan Miguel and I have ADHD. Since I have long wanted to
learn Ada but I could not learn, because the medication was not
correct and I could not concentrate properly. I returned to writing
simple programs to get base. All I want is to tell me how I am doing.
Thanks in advance.


with Ada.Text_IO;
with Ada.Numerics.Discrete_Random;
with Ada.Strings.Fixed;

use Ada;
use Ada.Strings.Fixed;

procedure ordenar_3_numeros_better is

	type UnoDiez is range 1 .. 10;

	package Ruleta10 is new Ada.Numerics.Discrete_Random(UnoDiez);

	Gen10: Ruleta10.Generator;

begin

	Ruleta10.Reset(Gen10);
	for I in 1 .. 10 loop

   		Proceso_Principal: declare

			A		: UnoDiez := Ruleta10.Random(Gen10);
			B		: UnoDiez := Ruleta10.Random(Gen10);
			C		: UnoDiez := Ruleta10.Random(Gen10);

			First	: UnoDiez;
			Secon	: UnoDiez;
			Third	: UnoDiez;

   		begin

			Formateo_Salida_1: declare

				Separador1 : String := (UnoDiez'Width - UnoDiez'Image(A)'Length) *
' ';
				Separador2 : String := (UnoDiez'Width - UnoDiez'Image(B)'Length) *
' ';
				Separador3 : String := (UnoDiez'Width - UnoDiez'Image(C)'Length) *
' ';

			begin

				Text_IO.Put (
								Separador1 & UnoDiez'Image(A) & ","    &
								Separador2 & UnoDiez'Image(B) & ","    &
								Separador3 & UnoDiez'Image(C) & "  --"
							);
			end Formateo_Salida_1;

			if (A >= B) then
				if (A >= C) then

					First := A;
					if (B >= C) then

						-- First := A;
						Secon := B;
						Third := C;
					else

						-- First := A;
						Secon := C;
						Third := B;
					end if;
				else  -- C > A >= B

					First := C;
					Secon := A;
					Third := B;
				end if;
			elsif (B >= A) then
				if (B >= C) then
					First := B;
					if (A >= C) then

						-- First := B;
						Secon := A;
						Third := C;
					else

						-- First := B;
						Secon := C;
						Third := A;
					end if;
				else -- C > B >= A

					First := C;
					Secon := B;
					Third := A;
				end if;
			end if;

			Formateo_Salida_2: declare

				Separador1 : String := (UnoDiez'Width -
UnoDiez'Image(First)'Length) * ' ';
				Separador2 : String := (UnoDiez'Width -
UnoDiez'Image(Secon)'Length) * ' ';
				Separador3 : String := (UnoDiez'Width -
UnoDiez'Image(Third)'Length) * ' ';

			begin

				Text_IO.Put (
								Separador1 & UnoDiez'Image(First) & ","  &
								Separador2 & UnoDiez'Image(Secon) & ","  &
								Separador3 & UnoDiez'Image(Third)
							);
				Text_IO.New_Line;
			end Formateo_Salida_2;
      	end Proceso_Principal;
	end loop;
end ordenar_3_numeros_better;

Re: I need feedback

[Ludovic Brenta]

juanmiuk <juanmiuk@googlemail.com> writes:
> My name is Juan Miguel and I have ADHD. Since I have long wanted to
> learn Ada but I could not learn, because the medication was not
> correct and I could not concentrate properly. I returned to writing
> simple programs to get base. All I want is to tell me how I am doing.
> Thanks in advance.

You're doing great.  I think your next step will be to turn
Formateo_Salida_1 and Formateo_Salida_2 into one subprogram taking three
parameters.

Also, you do not need parentheses around conditions:

if (A >= B) then

can also be written as

if A >= B then

One last remark: personally, I have found that mixing the English
keywords of Ada with non-English names distracts me when reading the
source.  I prefer everything to be in English even though it is not my
native language; you should experiment with that and see if that helps
you concentrate.

-- 
Ludovic Brenta.

Re: I need feedback

[Pascal Obry]

Le 14/06/2011 23:50, Ludovic Brenta a �crit :
> One last remark: personally, I have found that mixing the English
> keywords of Ada with non-English names distracts me when reading the
> source.  I prefer everything to be in English even though it is not my
> native language; you should experiment with that and see if that helps
> you concentrate.

Same for me. I just cannot write an Ada program with French identifiers.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|    http://www.obry.net  -  http://v2p.fr.eu.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver keys.gnupg.net --recv-key F949BD3B

Re: I need feedback

[Yannick Duchêne (Hibou57)]

Le Tue, 14 Jun 2011 23:50:58 +0200, Ludovic Brenta  
<ludovic@ludovic-brenta.org> a écrit:
> One last remark: personally, I have found that mixing the English
> keywords of Ada with non-English names distracts me when reading the
> source.  I prefer everything to be in English even though it is not my
> native language; you should experiment with that and see if that helps
> you concentrate.
This also help any one with no knowledge of one particular language, to  
understand, and help.

If the convention is to use meaningful names, then that names should also  
be written in the language which is the most widely understood (seems a  
reasonable implication).


-- 
“Syntactic sugar causes cancer of the semi-colons.”  [Epigrams on  
Programming — Alan J. — P. Yale University]
“Structured Programming supports the law of the excluded muddle.” [Idem]
“c++; /* this makes c bigger but returns the old value */” [Anonymous]

Re: I need feedback

[Shark8]

> This also help any one with no knowledge of one particular language, to  
> understand, and help.
>
> If the convention is to use meaningful names, then that names should also  
> be written in the language which is the most widely understood (seems a  
> reasonable implication).

That is helpful but, and this is my opinion, it seems that that's what
verbose
comment can alleviate. Not being, say a statistician, should not
impede me from
naming a variable Sigma and then commenting what the standard
deviation is.

Re: I need feedback

[juanmiuk]

Well, Thank you for all suggestion, I think I follow all of them.
There you go the final version:


with Ada.Text_IO;
with Ada.Integer_Text_IO;
with Ada.Numerics.Discrete_Random;
with Ada.Strings.Fixed;

use Ada;
use Ada.Strings.Fixed;

procedure Sort_3_Numbers is

	subtype One_Ten is Integer range 1 .. 10;

	package Roulette10 is new Ada.Numerics.Discrete_Random(One_Ten);

	Gen10: Roulette10.Generator;

   WIDTH_NUM : constant One_Ten := One_Ten'Width;
   THE_BASE  : constant One_Ten := 10;

   procedure Output_Num
      ( The_Number : in     One_Ten;
        Width_Num  : in     One_Ten;
        The_Base   : in     One_Ten )
   is

   begin

      Integer_Text_IO.Put(The_Number, Width_Num, The_Base);

   end Output_Num;

begin

	Roulette10.Reset(Gen10);
	for I in One_Ten'Range loop

   		Main_Process : declare

			A		: One_Ten := Roulette10.Random(Gen10);
			B		: One_Ten := Roulette10.Random(Gen10);
			C		: One_Ten := Roulette10.Random(Gen10);

			First	: One_Ten;
			Secon	: One_Ten;
			Third	: One_Ten;

   		begin

            Output_Num(A, WIDTH_NUM, THE_BASE);
            -- Changing all the Output_Num by Integer_Text_IO.Put(A,
WIDTH_NUM, THE_BASE)
            -- as Jeffrey Carter comments then won't be any
dupplications.
            Text_IO.Put(", ");
            Output_Num(B, WIDTH_NUM, THE_BASE);
            Text_IO.Put(", ");
            Output_Num(C, WIDTH_NUM, THE_BASE);
            Text_IO.Put("  --");

			   if (A >= B) then
			   	if (A >= C) then

			   		First := A;
			   		if (B >= C) then

			   			-- First := A;
			   			Secon := B;
			   			Third := C;
			   		else

			   			-- First := A;
			   			Secon := C;
			   			Third := B;
			   		end if;
			   	else  -- C > A >= B

			   		First := C;
			   		Secon := A;
			   		Third := B;
			   	end if;
		   	elsif (B >= A) then
		   		if (B >= C) then
		   			First := B;
		   			if (A >= C) then

						-- First := B;
		   				Secon := A;
		   				Third := C;
		   			else

		   				-- First := B;
		   				Secon := C;
		   				Third := A;
		   			end if;
		   		else -- C > B >= A

		   			First := C;
		   			Secon := B;
		   			Third := A;
		   		end if;
		   	end if;

            Output_Num (First, WIDTH_NUM, THE_BASE);
            Text_IO.Put(", ");
            Output_Num (Secon, WIDTH_NUM, THE_BASE);
            Text_IO.Put(", ");
            Output_Num (Third, One_Ten'Width, 10);
            Text_IO.New_Line;

      	end Main_Process;
	end loop;
end Sort_3_Numbers;

Re: I need feedback

[Ludovic Brenta]

juanmiuk wrote:
> Well, Thank you for all suggestion, I think I follow all of them.
> There you go the final version:
>
> with Ada.Text_IO;
> with Ada.Integer_Text_IO;
> with Ada.Numerics.Discrete_Random;
> with Ada.Strings.Fixed;
>
> use Ada;
> use Ada.Strings.Fixed;
>
> procedure Sort_3_Numbers is
>
>         subtype One_Ten is Integer range 1 .. 10;
>
>         package Roulette10 is new Ada.Numerics.Discrete_Random(One_Ten);
>
>         Gen10: Roulette10.Generator;
>
>    WIDTH_NUM : constant One_Ten := One_Ten'Width;
>    THE_BASE  : constant One_Ten := 10;

This is not necessary, see why below.

>    procedure Output_Num
>       ( The_Number : in     One_Ten;
>         Width_Num  : in     One_Ten;
>         The_Base   : in     One_Ten )
>    is
>
>    begin
>
>       Integer_Text_IO.Put(The_Number, Width_Num, The_Base);
>
>    end Output_Num;

This entire procedure does nothing but call Integer_Text_IO.Put, so is
not necessary; you might as well call Integer_Text_IO.Put directly.
In the spec of Integer_Text_IO.Put, the parameter Base has a default
value of 10, so you need not specify it.  You could even do away with
Width_Num like this:

package One_Ten_IO is new Ada.Text_IO.Integer_IO (Num => One_Ten);

This would initialize the constant One_Ten_IO.Default_Width to be
One_Ten'Width; you would not have to do it yourself.

> begin
>    Roulette10.Reset(Gen10);
>    for I in One_Ten'Range loop
>       Main_Process : declare
>          A               : One_Ten := Roulette10.Random(Gen10);
>          B               : One_Ten := Roulette10.Random(Gen10);
>          C               : One_Ten := Roulette10.Random(Gen10);
>
>          First   : One_Ten;
>          Secon   : One_Ten;
>          Third   : One_Ten;
>       begin
>          Output_Num(A, WIDTH_NUM, THE_BASE);
>          -- Changing all the Output_Num by Integer_Text_IO.Put(A, WIDTH_NUM, THE_BASE)
>          -- as Jeffrey Carter comments then won't be any dupplications.

Or, even more succinctly:

           One_Ten_IO.Put (A);

[rest snipped].

Hope this helps.

--
Ludovic Brenta.

Re: I need feedback

[Jeffrey Carter]

In addition to Brenta's comments, I would suggest that identifiers such as 
ordenar_3_numeros_better and Unodiez are more commonly written as 
Ordenar_3_Numeros_Better and Uno_Diez.

Also, using tabs to indent is not common practice, and makes your code difficult 
to read. I use 3 spaces; 2-4 spaces is common.

-- 
Jeff Carter
"Blessed are they who convert their neighbors'
oxen, for they shall inhibit their girth."
Monty Python's Life of Brian
83