Rzymskie na Arabskie

0

Witam, w podanym programie problem polega na tym, że składnia jest dobra ale nie rozumiem dla czego, otrzymuje błędne wyniki. Podaje poniżej kod sformatowany i mam nadzieję, że w końcu dojdziemy do porozumienia z pozdrowieniami dla -123oho. Napisałem komentarze z objaśnieniem, mam nadzieję, że do zrozumienia, z góry dziękuje.

 unit Unit1;

interface

uses
  Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
  Dialogs, StdCtrls;

type
  TForm1 = class(TForm)
    Edit1: TEdit;
    Button1: TButton;
    Edit2: TEdit;
    procedure Button1Click(Sender: TObject);
  private
    { Private declarations }
  public
    { Public declarations }
  end;

var
  Form1: TForm1;

implementation

{$R *.dfm}

procedure TForm1.Button1Click(Sender: TObject);
var i,dlugosc_lancucha,c_mnoznik,a_wynik_tymczasowy,s_wynik_koncowy,p,r:integer;
var jedynka,vpiec,dziesiec:char;
var t:array[1..20]of string;  //tablica przechowywujaca cyfry rzymskie
begin
c_mnoznik:=1;
r:=1;
dlugosc_lancucha:=length(edit1.text);


for i:=dlugosc_lancucha downto 1 do t[i]:=copy(edit1.Text,dlugosc_lancucha-(i-1),1);
//Powyżej wypelnianie tablicy tak, aby byly ustawione od konca
for p:=1 to dlugosc_lancucha do
  begin
 //zmienne p, i oraz r są po to, aby pomijac niektore kroi w tym forze
  i:=p;
  if(i=r)then
    begin  // warunek, ktory pomija dzialania w razie koniecznosci ich pominiecia
  {ponizej sprawdzanie wielkosci mnoznika. Jest on potrzebny do poprawnego dzialania programu.
  Program dziala tak:
    1. c_mnoznik=1 czyli sprawdzamy czy ostatna liczba jest jednostka, podana tutaj zmienna jest
      odpowiedzialna za przyrownanie bierzacego elementu do odpowiedniego zestawu znakow i pod
      koniec pomnozeniu a_wynik_tymczasowy przez nia. (np c_mnoznik=2, for bierze drugi zestaw
      znakow, t[i]='L' then a_wynik_tymczasowy:=5 i pod koniec a_wynik_koncowy:=a_wynik_koncowy
      *c_mnoznik przez co otrzymujemy wynik 50)
    2. Jesli ostatnia liczba to jednostka to warunki w ponizszym forze sprawdzaja jaka
    3. Jesli ostatnie liczba nie zgadza sie z zestawem znakow jedosci wtedy idziemy na koniec,
      powiekszamy c_mnoznik*10 co oznacza, ze w kolejnym kroku petli wezmiemy zestaw znakow dla
      dziesiatek i sprawdzimy czy liczba jest dziesiatka
    4. Jesli powyzszy pkt nie wykaze, ze liczba jest jednoscia, dziesiatka czy setka itd to znaczy,
      ze dane sa zle podane i nic nie zostanie podane
    5. Jesli wszystkie liczby zostaly rozpoznane to otzymujemy wynik za ktory odpowiedzialna jest
      zmienna s_wynik_kancowy
      }
  if(c_mnoznik=1)then
  begin
    jedynka:='I';
    vpiec:='V';
    dziesiec:='X';
  end;
  if(c_mnoznik=10)then
  begin
    jedynka:='X';
    vpiec:='L';
    dziesiec:='C';
  end;
  if(c_mnoznik=100)then
  begin
    jedynka:='C';
    vpiec:='D';
    dziesiec:='M';
  end;
  if(t[i]=jedynka)then
  begin    //jesli ostatni symbol zgadza sie z jednostka jednosci to...
    a_wynik_tymczasowy:=1;                   // a_wynik_tymczasowy:=1 a tymczasowy bo...
    if(t[i+1]=jedynka)then
    begin    // jesli jest I i przed nia jest kolejna I to jest to nadal cyfra jednosci
      a_wynik_tymczasowy:=2;                    // a jesli I wystepuje po poprzenim znaku to a_wynik_tymczasowy powinien byc =2
      if(t[i+2]=jedynka)then
      begin     // ale jelsi znow przed powyzsza bedzie I to ...
        a_wynik_tymczasowy:=3;                      //a_wynik_tymczasowy bedzie musial wynosci 3
        if(t[i+3]=vpiec)then
        begin       //ale jesli przed powyzszym symbolem bedzie V to ...
          a_wynik_tymczasowy:=8;                       //a_wynik_tymczasowy bedzie musial byc rowny 8 bo wyjdzie na to, ze wyglada on tak: VIII
          i:=i+1;                     //zwiekszam indeks po kazdym takim spelnionym warunku, poniewaz jesli wystepowala 8(VIII}
        end;                          //to kolejnym sprawdzanym elementem nie moze byc t[2], my juz jednosci otrzymalismy wiec
        i:=i+1;                       //kolejny elementy to musi byc cos wiekszego np dziesiatka, czyli kolejny sprawdzany
      end;                            //element to t[5] w tym przypadku. Po tych dzialaniach konczy sie pierwszy krok w petli
      if(t[i+2]=vpiec)then
      begin          // wiec c_mnoznik (jak uwyzej opisalem) jest zwiekszony o x10 co sprawia, ze kolejny zestaw
        a_wynik_tymczasowy:=7;                         //znakow bedzie dla dziesiatek - czyli tak jak powinno byc
        i:=i+1;
      end;
      i:=i+1;
    end;
    if(t[i+1]=vpiec)then
    begin
      a_wynik_tymczasowy:=6;
      i:=i+1;
    end;
  end;
  if(t[i]=vpiec)then
  begin
    a_wynik_tymczasowy:=5;
    if(t[i+1]=jedynka)then
    begin
      a_wynik_tymczasowy:=4;
      i:=i+1;
    end;
  end;
  if(t[i]=dziesiec)then
  begin
    if(t[i+1]=jedynka)then
    begin
      a_wynik_tymczasowy:=9;
      i:=i+1;
    end;
  end;  //tytaj do konca tych warunkow byly sprawdzane wszystkie mozliwe wystepowanie liczb
  a_wynik_tymczasowy:=a_wynik_tymczasowy*c_mnoznik;  //a_wynik_tymczasowy*c_mnoznik dla liczb wiekszych lub rownych 10
  s_wynik_koncowy:=s_wynik_koncowy+a_wynik_tymczasowy;    //koncowy wynik laczacy a_wynik_tymczasowy z wszystkich korkow petli
  edit2.Text:=inttostr(s_wynik_koncowy);   //wypisanie rezultatow
  c_mnoznik:=c_mnoznik*10;      //zwiekszenie c_mnoznik x10 powyzej wytlumaczone why
  r:=p;        //mnienna pozwalajaca na pomijanie krokow fora
  end;
  end;



end;

end.
1

var t:array[1..20]of string;

Jak to przechowuje cyfry, skoro jest string. Powinno być raczej char, no ale teraz nie zajmujmy się optymalizacją!

Twoje formatowanie jest żałosne ale nie martw się, nawet jak Lazarusowi to dałem to nic ciekawego nie wypluł bo kod jest po prostu nieczytelny, już nie mówiąc o obfuskacji w postaci komentarzy.

r := p; //mnienna pozwalajaca na pomijanie krokow fora

No to czemu nie nazwiesz tego normalnie?!

a_wynik_tymczasowy := 1; // a_wynik_tymczasowy:=1 a tymczasowy bo...

Ale przydatny komentarz! Koniecznie trzeba było powtórzyć kod bo przecież inaczej byśmy nie wiedzieli.

a_wynik_tymczasowy := 3;
//a_wynik_tymczasowy bedzie musial wynosci 3

Tak, koniecznie przekrocz limit 80 znaków wobec czego formater przejdzie do nowej linii, me gusta. Już o przydatności komentarza nie mówiąc!

Jest on potrzebny do poprawnego dzialania programu.

To ten program działa poprawnie?

Napisałem komentarze z objaśnieniem, mam nadzieję, że do zrozumienia

Niestety nie. Ten kod to jedna wielka kupa. 0 procedur, 0 podzielenia problemu na mniejsze, natomiast masa mylenia czym się tylko da. Tego kodu nie da się poprawić, go trzeba napisać od nowa.
Nie wiem jak wpadłeś na tak szatańskie rozwiązanie problemu które na dodatek nie działa, nie mniej utrudniasz rzeczy na siłę. Na wikipedii jest wyraźnie napisane że jeżeli liczba mniejsza znajduje się przed większą to odejmujesz i skaczesz o dwie liczby a w innym przypadku po prostu dodajesz. Nie wiem po co kombinujesz z nie wiadomo czym...
Implementacja konwersji liczb rzymskich w RTLu FPC ma 50 linii, czy ty naprawdę musisz utrudniać?

Coś ci poradzę: Napisz ten kod od nowa, nie kombinując tylko używając TABLICY która zawiera znaki i potem porównując po indeksach (czyli: 1.pobierz wartość aktulanego znaku 2.pobierz wartość następnego znaku 3. następny większy? Odejmuj i skacz o dwa znaki 4. nie większy? To dodaj do wyniku i skacz o jeden znak.)

0

:|
Nie wiem po co to tak komplikujesz, nawet nie zamierzam sprawdzać czy to się kompiluje, aby kompilator nie dostał focha.
http://delphi.about.com/od/delphitips2009/qt/roman-arabic.htm
Tutaj masz to logicznie napisane i co najważniejsze - przejrzyście!

0

Dam gotowca w C++. Przyjmuje on, że jeżeli obecny 'składnik' jest mniejszy niż następny to trzeba go od sumy odjąć, a przeciwnym wypadku dodać. Dla "" zwraca 0.

#include <string>
#include <map>
using namespace std;

static map<char,int> createValues() {
    map<char, int> val;
    val['I'] = 1;
    val['V'] = 5;
    val['X'] = 10;
    val['L'] = 50;
    val['C'] = 100;
    val['D'] = 500;
    val['M'] = 1000;
    return val;
}

int romanToInt(const string &literal) {
    static const map<char,int> values = createValues();
    if (literal == "") return 0;

    int len = literal.length();
    int sum = values.at(literal[len-1]);
    for (int i = 0; i < len-1; i++) {
        int current = values.at(literal[i]);
        int next = values.at(literal[i+1]);

        sum += ((current < next)  ?  -current  :  current);
    }
    return sum;
}
2

Ten kod jest faktycznie nieziemsko nieczytelny, a jeszcze te komentarze... Nie przejmuj się, w końcu albo sam dojdziesz do rozwiązania, albo ktoś je znajdzie za Ciebie :]

Jeżeli kiedykolwiek napiszesz kod, który nie działa tak, jak należy to postaraj się analizować go etapami; Masz do tego tak rewelacyjne narzędzie jak Debugger, dzięki któremu można znaleźć 99.99% błędów w kodzie programu; Przede wszystkim trzymaj się zasady DRY i KISS, a także rozkładaj kod na procedury m.in. po to, by czytelność była większa (o innych aspektach pomyśl sam); Analizuj kod krok po kroku używając do tego okna Watches - podpowie Ci jakie wartości mają konkretne zmienne;


Trochę teorii nie zaszkodzi, teraz zabierzmy się za kod; Z racji tej, ze pole kodu w poście jest dość wązkie (u mnie) - kod analizuję w kompilatorze; Niestety z formatowaniem kodu u Ciebie nie najlepiej, postaram się go sformatować po swojemu, żeby wszyscy widzieli o co w nim chodzi;


Sformatowany kod procedury TForm1.Button1Click:

procedure TForm1.Button1Click(Sender: TObject);
var
  i, dlugosc_lancucha, c_mnoznik, a_wynik_tymczasowy, s_wynik_koncowy, p, r: Integer;
  jedynka, vpiec, dziesiec: Char;
  t: array [1 .. 20] of String;
begin
  c_mnoznik := 1;
  r := 1;
  dlugosc_lancucha := Length(Edit1.Text);

  for i := dlugosc_lancucha downto 1 do
    t[i] := Copy(Edit1.Text, dlugosc_lancucha - (i - 1), 1);

  for p := 1 to dlugosc_lancucha do
  begin
    i := p;

    if i = r then
    begin
      if c_mnoznik = 1 then
      begin
        jedynka  := 'I';
        vpiec    := 'V';
        dziesiec := 'X';
      end;

      if c_mnoznik = 10 then
      begin
        jedynka  := 'X';
        vpiec    := 'L';
        dziesiec := 'C';
      end;

      if c_mnoznik = 100 then
      begin
        jedynka  := 'C';
        vpiec    := 'D';
        dziesiec := 'M';
      end;

      if t[i] = jedynka then
      begin
        a_wynik_tymczasowy := 1;

        if t[i + 1] = jedynka then
        begin
          a_wynik_tymczasowy := 2;

          if t[i + 2] = jedynka then
          begin
            a_wynik_tymczasowy := 3;

            if t[i + 3] = vpiec then
            begin
              a_wynik_tymczasowy := 8;
              Inc(i);
            end;

            Inc(i);
          end;

          if t[i + 2] = vpiec then
          begin
            a_wynik_tymczasowy := 7;
            Inc(i);
          end;

          Inc(i);
        end;

        if t[i + 1] = vpiec then
        begin
          a_wynik_tymczasowy := 6;
          Inc(i);
        end;
      end;

      if  t[i] = vpiec then
      begin
        a_wynik_tymczasowy := 5;

        if t[i + 1] = jedynka then
        begin
          a_wynik_tymczasowy := 4;
          Inc(i);
        end;
      end;

      if t[i] = dziesiec then
        if t[i + 1] = jedynka then
        begin
          a_wynik_tymczasowy := 9;
          Inc(i);
        end;

      a_wynik_tymczasowy := a_wynik_tymczasowy * c_mnoznik;
      Inc(s_wynik_koncowy, a_wynik_tymczasowy);
      Edit2.Text := IntToStr(s_wynik_koncowy);
      c_mnoznik := c_mnoznik * 10;
      r := p;
    end;
  end;
end;

Dodatkowo usunąłem jeden niepotrzebny blok begin .. end; Dalej masz dużo zmiennych, które nic o swoim przeznaczeniu nie mówią - trzeba analizować cały kod żeby się domyślić do czego służą... Postaram się trochę skrócić ten kod przez zastosowanie kilku prostych zabiegów;


      if c_mnoznik = 1 then
      begin
        jedynka  := 'I';
        vpiec    := 'V';
        dziesiec := 'X';
      end;

      if c_mnoznik = 10 then
      begin
        jedynka  := 'X';
        vpiec    := 'L';
        dziesiec := 'C';
      end;

      if c_mnoznik = 100 then
      begin
        jedynka  := 'C';
        vpiec    := 'D';
        dziesiec := 'M';
      end;

Tutaj aż prosi się o zastosowanie bloku case .. of z racji tej, że każdy z tych warunków zostanie sprawdzonych:

      case c_mnoznik of
        1:   begin
               jedynka  := 'I';
               vpiec    := 'V';
               dziesiec := 'X';
             end;
        10:  begin
               jedynka  := 'X';
               vpiec    := 'L';
               dziesiec := 'C';
             end;
        100: begin
               jedynka  := 'C';
               vpiec    := 'D';
               dziesiec := 'M';
             end;
      end;

i już kod wygląda prościej a spełnia tą samą rolę; Uparty może jeszcze bardziej go skrócić wykorzystując macierz ze znakami:

const
  aSymbols: array [0 .. 6] of Char = ('I', 'V', 'X', 'L', 'C', 'D', 'M');
var
  iOffset: Byte;

  {...}
begin
  {...}

  iOffset := (Length(IntToStr(c_mnoznik)) - 1) * 2;

  jedynka  := aSymbols[iOffset];
  vpiec    := aSymbols[iOffset + 1];
  dziesiec := aSymbols[iOffset + 2];

i jeszcze mniej pisania dzięki macierzy i prostej arytmetyce;


Reszty kodu nie będę Ci optymalizował bo sam się w tych warunkach gubię... Teraz zastanów się, czy konieczne jest pisanie tak zawiłych warunków na rzecz dość prostych operacji;

Polecam usiąść, wziąć kartkę i długopis, rozpisać sobie krok po kroku (np. w pseudojęzyku) kolejne instrukcje, jakie trzeba wykonać by przekonwertować daną liczbę na łańcuch; Inaczej będziesz biegał oczami po kodzie i dalej nic nie będziesz z niego wiedział, a tym bardziej nie znajdziesz błędów (a jak nie użyjesz debuggera to na 99% ich nie znajdziesz);
Część (odnośnie macierzy) już Ci podpowiedziałem (jeżeli w ogóle będziesz chciał poprawiać swój kod), resztę postaraj sie samemu przeanalizować i ocenić, czy jest sens aż tak komplikować kod; Może jak będę miał chwilkę to nabazgrole taki algorytm, ale kodu Ci nie dam - musisz go napisać sam, a jak napiszesz go i będzie działał to może swoim się także pochwalę;

W każdym razie zabieraj sie do planowania i testowania nowego algorytmu, póki jest na tyle krótki, że można go napisac od nowa; Nie zapomnij o prostocie konstrukcji i formatowaniu kodu na bieżąco;

0

Rozumiem, sposób taki bo tak sobie to obmyśliłem, oczywiście da się szybciej. Program tak napisałem dla siebie, dla własnego sprawdzenia. Dzięki za pomocne info, no może do tego dojdę. Faktycznie da się jeszcze kod bardziej sformatować, o niektórych metodach nawet bym nie pomyślał (np. case). Tak na przyszłość to po prostu muszę się przyzwyczaić, że jak zrobię nowy temat na forum, to wielki -123oho będzie wszystko krytykował cytując moje zdania i wytykając najmniej istotne gafy jak przekręcenie jednej litery (jak co temat), i niestety dla ciebie -123oho, nie zrezygnuje z programowania, zbyt błachym powodem byś był, jesteś tylko jednym z tysięcy użytkowników. Do kodu wracając, dziękuje za pomoc, napiszę od nowa innym sposobem ale o tym jeszcze pomyślę, może jeszcze trochę teorii i formatowanie też nadrobię, pozdrawiam.

0
TekMast napisał(a)

Rozumiem, sposób taki bo tak sobie to obmyśliłem, oczywiście da się szybciej.

Nie ważne na początek jaki długi będzie algorytm, ważne żeby działał; Nie raz jeszcze napiszesz jakiś program twierdząc, że szybszego nie da się zrobić, a po dwóch tygodniach zoptymalizujesz go o kilkanaście procent; Praktycznie każdy algorytm da się przyspieszyć/zoptymalizować, ale do tego potrzeba czasu;

TekMast napisał(a)

Faktycznie da się jeszcze kod bardziej sformatować, o niektórych metodach nawet bym nie pomyślał (np. case).

Da się - owszem, jednak kod formatuje się na bieżąco, a nie po skończeniu danego bloku; Musisz przyzwyczaić się do tego, bo inaczej przy większych projektach będziesz się w kółko motał nie mogąc znaleźć potrzebnych rzeczy;

Jeśli chodzi o takie elementy jak blok case .. of - polecam zapoznać się ze wszystkimi elementami języka, po to by tworzyć przemyślany, dobrze skonstruowany i efektywny kod; Znajomość tego typu rzeczy jest nieodzowna;

TekMast napisał(a)

Tak na przyszłość to po prostu muszę się przyzwyczaić, że jak zrobię nowy temat na forum, to wielki -123oho będzie wszystko krytykował cytując moje zdania i wytykając najmniej istotne gafy jak przekręcenie jednej litery

Każdy musi sie do tego przyzwyczaić - nie tylko Ty...

TekMast napisał(a)

Do kodu wracając, dziękuje za pomoc, napiszę od nowa innym sposobem ale o tym jeszcze pomyślę, może jeszcze trochę teorii i formatowanie też nadrobię

Napisz go na spokojnie, dobrze zastanów się i rozpisz go nawet w pseudojęzyku - będzie Ci łatwiej; Zapoznaj się także z kodem jaki podsunął Ci @Patryk27 - zobacz ile trzeba by napisac ten algorytm; Kombinuj i testuj a znajdziesz rozwiązanie;

1 użytkowników online, w tym zalogowanych: 0, gości: 1