Obsługa plików XML - zasady tworzenia metod i zdarzeń

0

Witam

Jestem świeży, zielony ucze się sam od zera i właśnie się ostro zakręciłem.

Dopóki jako parametr metody Load() dawałem konkretną scieżkę dostępu do pliku wszystko było ok, ale jak chciałem skorzystać z OpenFileDialog() kompletnie się pogubiłem nie wiem jak przekazac scieżkę dostępu którą otrzymuje w zdarzeniu kliknięcia przycisku, do metody mojej klasy DocumentDXML

moja klasa DocumentDXML

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Xml;
using System.IO;
using System.Windows.Forms;


namespace ManagerProgramowSOM
{
    class DocumentDXML
    {
        public int licznikWezlow;
        public XmlDocument DXMLDoc = new XmlDocument();              

        public void LadowaniePlikuDoPamieci()
        {            
            DXMLDoc.Load(sciezka);         //<-- dopóki zamiast zmiennej 'sciezka' była "D:\\programy\\plik.dxml" było wszytko ok.
            licznikWezlow = DXMLDoc.GetElementsByTagName("program").Count;
        }        
    }
} 

Form1.cs

namespace ManagerProgramowSOM
{
    public partial class Form1 : Form
    {
        DocumentDXML mojDokument = new DocumentDXML();

        public Form1()
        {
            InitializeComponent();
        }

        private void button4_Click(object sender, EventArgs e)
        {
            OpenFileDialog sciezka = new OpenFileDialog();
            sciezka.ShowDialog();
            
            if (sciezka.FileName != "")
            {
                textBox1.Clear();
                listBox1.Items.Clear();

                mojDokument.LadowaniePlikuDoPamieci();
                textBox1.Text = Convert.ToString(mojDokument.licznikWezlow);

                for (int i = 0; i < mojDokument.licznikWezlow; i++)
                {
                    listBox1.Items.Add(mojDokument.DXMLDoc.GetElementsByTagName("program").Item(i).Attributes["name"].Value.ToString());
                }                
            }
        }  
    }
}
 

Prosiłbym również o naprowadzenie, jak to powinno wyglądać bo mam przeczucię że ostro namieszałęm w zdarzeniu kliknięcia - wydaje mi się że więcej powinienem pakować do swojej klasy a w zdarzeniu tylko odwoływać się do jej metod ale jak zaczynam pakować wszytko w swojej klasie to nie wiem jak określać konkretne kontrolki np. listBox1 i textBox1.

0

Zrób sobie konstruktor tej Twojej klasy, w którym będziesz podawał jako jeden argument ścieżkę/URL do pliku xml, jaki chcesz wczytać, a potem już wykonuj potrzebne metody. btw skąd Twoja klasa ma wiedzieć, co to jest "ścieżka" ? Co najwyżej ścieżka.FileName, a musisz też sprawdzić czy DialogResult jest już.

1

Czyli musisz tam zrobić coś podobnego:

 
if(openFileDialog.ShowDialog() == DialogResult.OK)
    {
        try
        {
            zrób coś, np. utwórz instancję klasy, 
            DocumentDXML doc = new DocumentDXML(openFileDialog.FileName);
            doc.ZróbCoś();
        }
        catch (Exception ex)
        {
            MessageBox.Show("Problem:" + ex.Message);
        }
    }
}
0

Dzieki, w trakcie wykombinowalem jeszcze cos takiego:

moja klasa DokumentDxml.cs

 namespace ManagerProgramowSOM
{
    class DocumentDXML
    {
        public int licznikWezlow;
        public XmlDocument Doc = new XmlDocument();
        public OpenFileDialog sciezka = new OpenFileDialog();

        public void PobieranieSciezkiDoPliku()
        {
            sciezka.ShowDialog();   
        }


        public void LadowaniePlikuDoPamieci()
        {            
            Doc.Load(sciezka.FileName);
            licznikWezlow = Doc.GetElementsByTagName("program").Count;
        }        
    }
}

Form1.cs

 namespace ManagerProgramowSOM
{
    public partial class Form1 : Form
    {
        DocumentDXML mojDokument = new DocumentDXML();

        public Form1()
        {
            InitializeComponent();
        }

        public void button4_Click(object sender, EventArgs e)
        {
            mojDokument.PobieranieSciezkiDoPliku();          
            
            if (mojDokument.sciezka.FileName != "")
            {
                textBox1.Clear();
                listBox1.Items.Clear();
                
                mojDokument.LadowaniePlikuDoPamieci();
                textBox1.Text = Convert.ToString(mojDokument.licznikWezlow);

                for (int i = 0; i < mojDokument.licznikWezlow; i++)
                {
                    listBox1.Items.Add(mojDokument.Doc.GetElementsByTagName("program").Item(i).Attributes["name"].Value.ToString());
                }                
            }
        }
    }
}

Działa i teraz pytanie brzmi czy to jest zgodne ze sztuka pisania programów?

PS. Pytam bo chciałbym wiedziec co jest dobrym nawykiem. co mi sie pozniej bedzie bardziej przydawalo.

1

Źle!
if(openFileDialog.ShowDialog() == DialogResult.OK)
W ten sposób sprawdzasz czy dialog otwarcia pliku (czy zgodnie ze sztuką każdy inny), został zatwierdzony, kliknięty przycisk OK (potwierdzenia). Kliknięcie cancel wg mnie nie czyści FileName i może tam być jakaś wartość.
Poza tym dość optymistycznie podchodzisz do otwarcia pliku. Ktoś napisał o użyciu try-catch, możesz jeszcze przed jego otwarciem sprawdzić czy istnieje System.IO.File.Exists(path).
Klasa DocumentDXML to trochę lipa, bo mieszasz w niej warstwę logiki i prezentacji. Nie powinna ona zawierać OpenFileDialog. Warstwa prezentacji jest odpowiedzialna za to żeby zdobyć od usera tą scieżkę. Czymu zakładasz że przez openFileDialog to jedyna droga. A jak będziesz chciał z argumentów wywołania programu, z drag'n'drop, z rozpoznawania mowy, z rozpoznawania obrazu, ... Metod może być wiele. Klasy DocumentDXML nie obchodzi jak zdobyłeś ścieżkę, tylko jej zadaniem jest załadowanie pliku i wykonanie jakiś operacji.

Z dialogami zasada taka:
Tworzysz instancję.
Pokazujesz.
Zbierasz jej DialogResult, na którego podstawie podejmujesz kolejne akcje. http://msdn.microsoft.com/en-us/library/system.windows.forms.dialogresult.aspx
Zazwyczja OK lub Yes to sygnał że user chce żeby wykonać jakąś akcję o która pyta dialog.

0

dzieki massther coś zaczyna mi świtać, ale musze jeszcze trochę doczytać.

Czyli w swojej klasie musze pakować tylko warstwe logiki a warstwa prezentacji to to co jest w zdarzeniach w From1? Warstwa prezentacji komunikuje się przez instancję obiektu z metodami klasy obiektu czyli z logiką?

PS. Jak zaczynałem jakieś dwa miesiące temu się w to wgryzać to nie miałem pojęcia że w programowaniu trzeba tak wielowarstwowo myśleć i kojarzyć, szacun dla programistów! A myślałem do tej pory że najlepszy masaż mózgu gwarantują sudoku z Rozrywki, myliłem się.

1

Warstwa logiki też może "komunikować się" za pomocą zdarzeń. Może rzucać pewne zdarzenia (np. KoniecObliczeń, OdebrałamKomunikat), a warstwa prezentacji będzie na nie reagowała. Zdarzenia w warstwie prezentacji są głównie wynikiem interakcji z użytkownikiem. Ale to nie są jedyne formy komunikacji. Odpowiednie komponenty mogą wywoływać zwyczajnie jakieś metody, modyfikować właściwości, etc.

0

Zrobiłem to od nowa, prosiłbym o sprawdzenie czy teraz jest to poprawnie zorganizowane.

moja klasa DokumentDxml.cs

  class PlikProgramyDXML
    {
        private string sciezkaPliku;
        private static int iloscWezlow;
        private string[] tablicaNazwWezlow; 
        private XmlDocument Doc = new XmlDocument();
       
        public PlikProgramyDXML()
        {
            sciezkaPliku = null;
            iloscWezlow = 0;            
        }    

        public void ZaladowaniePliku(string path)
        {
            sciezkaPliku = path;
            Doc.Load(sciezkaPliku);
        }

        public int PobranieIlosciWezlow()
        {                       
            return iloscWezlow = Doc.GetElementsByTagName("program").Count;            
        }        

        public void WczytanieNazwWezlow()
        {
            tablicaNazwWezlow = new string[iloscWezlow];
            for (int i = 0; i < iloscWezlow - 1; i++)
            {
                tablicaNazwWezlow[i] = Doc.GetElementsByTagName("program").Item(i).Attributes["name"].Value.ToString();
            }            
        }

        public string[] PrzekazanieNazwWezlow()
        {            
            return tablicaNazwWezlow;
        }

        public void UsuwanieWezla()
        {
            //todo
        }

        public void DodanieWezla()
        {
            //todo
        }
    }

Form1.cs

 public void button4_Click(object sender, EventArgs e)
        {
            textBox1.Clear();
            listBox1.Items.Clear();
            PlikProgramyDXML lewyPlik = new PlikProgramyDXML();
            
            OpenFileDialog openFileDialogLewy = new OpenFileDialog();            
            if (openFileDialogLewy.ShowDialog() == DialogResult.OK)
            {
                try
                {
                    lewyPlik.ZaladowaniePliku(openFileDialogLewy.FileName);
                    textBox1.Text = Convert.ToString(lewyPlik.PobranieIlosciWezlow());
                    lewyPlik.WczytanieNazwWezlow();

                    for (int i = 0; i < (lewyPlik.PobranieIlosciWezlow()-1); i++)
                    {
                        listBox1.Items.Add(lewyPlik.PrzekazanieNazwWezlow()[i]);
                    } 
                }
                catch (Exception ex)
                {
                    MessageBox.Show("Błąd: Nie można odczytać pliku z dysku. Szczegóły błędu: " + ex.Message);
                }
            }
        }
private void wczytaj_plik_prawy_button_Click(object sender, EventArgs e)
        {
            textBox4.Clear();
            listBox2.Items.Clear();
            PlikProgramyDXML prawyPlik = new PlikProgramyDXML();

            OpenFileDialog openFileDialogPrawy = new OpenFileDialog();
            if (openFileDialogPrawy.ShowDialog() == DialogResult.OK)
            {
                try
                {
                    prawyPlik.ZaladowaniePliku(openFileDialogPrawy.FileName);
                    textBox4.Text = Convert.ToString(prawyPlik.PobranieIlosciWezlow());
                    prawyPlik.WczytanieNazwWezlow();

                    for (int i = 0; i < (prawyPlik.PobranieIlosciWezlow() - 1); i++)
                    {
                        listBox2.Items.Add(prawyPlik.PrzekazanieNazwWezlow()[i]);
                    }
                }
                catch (Exception ex)
                {
                    MessageBox.Show("Błąd: Nie można odczytać pliku z dysku. Szczegóły błędu: " + ex.Message);
                }
            }
        }                           

Docelowo program ma dzialac na zasadzie otwierania dwóch plików XML, jeden w lewym panelu drugi w prawym i przenoszeniu lub kopiowaniu pojedynczych wezlów pomiedzy tymi plikami, stad nazwa obiektu lewyPlik.

1

Całkiem nieźle. Kilka kosmetyczny uwag.
PobranieIlosciWezlow() zrobiłbym to jako właściwość, która wylicza się po ZaladowaniePliku i zmienia wartość po DodanieWezla i UsuwanieWezla. Myślę że będzie to wydajniejsze niż za każdym wywołaniem metody wyliczanie liczby węzłów.
W WczytanieNazwWezlow przydałby się jakiś try-catch, bo jeśli nie będzie tagu program, albo atrybutu name to się wysypie. Także ...Value.ToString() jest groźne, bo jeśli Value będzie null, to oberwiesz exception'em.
Czemu w for masz iloscWezlow - 1? Przecież jak masz dwa węzły, to mają indexy 0 i 1, więc form wygląda tak: for (int i = 0; i < iloscWezlow; i++)
Jeśli catch (Exception ex) { MessageBox.Show("Błąd: Nie można odczytać pliku z dysku. Szczegóły błędu: " + ex.Message); } ma służyć tylko do informowania że nie można załadować pliku to try powinien być tylko na prawyPlik.ZaladowaniePliku(openFileDialogPrawy.FileName);
PrzekazanieNazwWezlow też mogłoby być właściwością.
WczytanieNazwWezlow() powinno być metodą prywatną i być wołane w ZaladowaniePliku. Co cię na zewnątrz tej klasy obchodzi jaki jest algorytm wczytania węzłów, czy generalnie przetworzenia pliku. Twoja klasa ma udostępnić liczbę węzłów i ich nazwy. Więc niech się tym zajmie. Form1 tylko to interesuje.

0

Dzięki wielkie za podpowiedzi, wszystko zanotowane i będe dążył do zrobienia tak jak mówisz, ale musze jeszcze doczytać w literaturze.

EDIT:
{
Już się dopatrzyłem, w bloku try { } mam mieć tylko linie wczytywania plików, ładowanie wartości do textBoxów i pobieranie nazw węzłów umieszczam poza blokiem try { }.
}

Wyjaśnij jeszcze tylko jak możesz co masz na myśli pisząc:

massther napisał(a)

(...)
Jeśli catch (Exception ex) { MessageBox.Show("Błąd: Nie można odczytać pliku z dysku. Szczegóły błędu: " + ex.Message); } ma służyć tylko do informowania że nie można załadować pliku to try powinien być tylko na prawyPlik.ZaladowaniePliku(openFileDialogPrawy.FileName); (...)

Nie rozumiem dlaczego try mam stosować tylko do prawego pliku.
Oba pliki są (będą) otwierane/zapisywane niezaleznie przez różne przyciski, Form1 wygląda tak:
user image

0

Chodziło mi że blok try objął nie tylko wczytanie z pliku, ale też wczytywanie nazw wezłów etc. Natomiast wiadomość wyświetlana w bloku catch sugeruje że problem był w ładowaniu pliku. Ale błąd mógł wystąpić podczas przetwarzania poprawnie załadowanego xml, czy dodawania do listbox. Wtedy twój komunikat jest mylący.

0

Mam problem ze napisaniem metody, wstawiania skopiowanego, wypełnionego danymi, węzła z jednego pliku XML do drugiego pliku XML.

Metoda kopiowania wezla dziala:

        public XmlNode KopiujProgram(string zaznaczony)
        {            
            XmlNode kopiaProgramu = null;
            kopiaProgramu = Doc.SelectSingleNode("/device/programs/program[@name='" + zaznaczony + "']");     //do kopiaProgramu łąduje wezel z Doc wyszukany po nazwie programu (po atrybucie 'name')    
            return kopiaProgramu;
        }//OK 

Metoda usuwania wezla, działa również:

 
        public void UsunProgram(string zaznaczony)
        {            
            XmlNode zaznaczonyWezel = null;                            
            zaznaczonyWezel = Doc.SelectSingleNode("/device/programs/program[@name='" + zaznaczony + "']");                
            zaznaczonyWezel.ParentNode.RemoveChild(zaznaczonyWezel); //usuwam wezel wraz ze znacznikami <program>...</program>

            this.IloscWezlow = Doc.GetElementsByTagName("program").Count;
            this.WczytanieNazwWezlow();
        }//OK

Analogicznie do tych powyzej próbuje napisać metodę wstawiania wezła do Doc ale zwracami mi błędy, na razie wygląda tak:

         public void WstawProgram(XmlNode program)
        {
            XmlNode ostatniWezelProgram = null;
            XmlNode wezelNadrzednyPrograms;
            wezelNadrzednyPrograms = Doc.SelectSingleNode("/device/programs");
            ostatniWezelProgram = wezelNadrzednyPrograms.LastChild;
            wezelNadrzednyPrograms.InsertAfter(program, ostatniWezelProgram) // tutaj wyskakuje mi błąd "Węzeł, który ma zostać wstawiony, pochodzi z innego kontekstu dokumentu."
            //wezelNadrzednyPrograms.AppendChild(program); //tutaj tez to samo

            this.IloscWezlow = Doc.GetElementsByTagName("program").Count;
            this.WczytanieNazwWezlow();
        }//Nie OK 

Nie wiem jaką metodę zastosować do wstawiania węzła, nie jestem tez pewien czy wartość typu XmlNode zwracana przez metode KopujProgram() jest odpowiedniego typu jako parametr formalny do metody WstawProgram().

Prosiłbym o przynajmniej napisanie pseudokodu co po kolei musze zrobic zeby poprawnie wstawic wezel, na razie próbowałem takich sposobów:

znaleŹĆ wezel nadrzedny do wezla ktory ma byc wstawiony
wezlowi nadrzednemu dodac "dziecko" i temu dziecku przypisac wartosc wstawianego programu.

//kod powyzej 

znalezc wezel nadrzedny do wezla ktory ma byc wstawiony
utworzyc w nim pusty wezel <program></program>
dodac atrybuty do tego wezla,
uzupelnic atrybuty, wstawiajac im wartosci pobrane z wezla ktory ma byc wstawiony
dodac podwezly skopiowane z wezla ktory ma byc wstawiony

        public void WstawProgram(XmlNode program,string zaznaczony)
        {            
            XmlNode programNode = Doc.CreateElement("program");
            XmlAttribute programAtrybut1 = Doc.CreateAttribute("name");
            XmlAttribute programAtrybut2 = Doc.CreateAttribute("service_program");
            XmlAttribute programAtrybut3 = Doc.CreateAttribute("visible");
            programAtrybut1.Value = zaznaczony;
            programAtrybut2.Value = "False";
            programAtrybut3.Value = "True";
            programNode.Attributes.Append(programAtrybut1);
            programNode.Attributes.Append(programAtrybut2);
            programNode.Attributes.Append(programAtrybut3);
            programNode = program;   //nie wywala bledu ale nie widze efektow nic sie nei dzieje, nie aktualizuje mi wlasciwosci ilosc wezlow i nazw wezlow tym bardziej wartosci wezlow
 
            //for (int i = 0; i < program.ChildNodes.Count; i++)
            //{
            //   programNode.AppendChild(program.ChildNodes[i]);   //tu zwracalo mi blad "Węzeł, który ma zostać wstawiony, pochodzi z innego kontekstu dokumentu."
            //}

            this.IloscWezlow = Doc.GetElementsByTagName("program").Count;
            this.WczytanieNazwWezlow();
        } 
  1. tu juz sie zaczynaja desperacyjne proby na chybil trafil ze stosowaneim metod ktore podpowiada intelisense,i co do ktorych kompilator sie nie burzy.
 
            listaProgramow = Doc.SelectNodes("/device/programs/program");
            int licznik = listaProgramow.Count;
            XmlNode rodzic = Doc.SelectSingleNode("/device/programs");
            rodzic.AppendChild(program).InsertAfter(program, listaProgramow[licznik]);
 
            XmlNode rodzic = Doc.SelectSingleNode("/device/programs");
            rodzic.LastChild.AppendChild(program);
  1. ponizszy przyklad w koncu zadzialal, dodal wezel, ale zawartosc wezla raz mi sie posypala, znaczniki "nagłówka" wezla i sam nagłówek był OK, ale to pomiedzy było przy przeglądaniu notatnikiem wszystko ciągiem, znacznimi podwezlow wezla <program> czyli nawiasy < i > byly zastapione jakims ciagiem znakow teraz nie pamietam konktretnie ale to bylo cos &gl i &cos tam
 

        public void WstawProgram(XmlNode program, string zaznaczony)
        {
            XmlElement elem = Doc.CreateElement("program");
            elem.InnerXml = program.InnerXml;   //chyba ta linia jest odpowiedzialna za taki blad ale pewien nie jestem, drugi raz nie moge go wywolac.

            XmlAttribute programAtrybut1 = Doc.CreateAttribute("name");
            XmlAttribute programAtrybut2 = Doc.CreateAttribute("service_program");
            XmlAttribute programAtrybut3 = Doc.CreateAttribute("visible");
            programAtrybut1.Value = zaznaczony;
            programAtrybut2.Value = "False";
            programAtrybut3.Value = "True";
            elem.Attributes.Append(programAtrybut1);
            elem.Attributes.Append(programAtrybut2);
            elem.Attributes.Append(programAtrybut3);
            
                XmlNode ostatniWezelProgram = null;
                XmlNode wezelNadrzednyPrograms;
                wezelNadrzednyPrograms = Doc.SelectSingleNode("/device/programs");
                ostatniWezelProgram = wezelNadrzednyPrograms.LastChild;
                //wezelNadrzednyPrograms.AppendChild(program);
                wezelNadrzednyPrograms.InsertAfter(elem, ostatniWezelProgram);
            
            this.IloscWezlow = Doc.GetElementsByTagName("program").Count;
            this.WczytanieNazwWezlow();
        }//Nie OK

Prosilbym o podpowiedz, jak to zrobic?
ponizej schemat moich plikow xml:

 
<?xml version="1.0" encoding="windows-1250"?>
<device name="NazwaMaszyny">
  <programs>

    <program name="NazwaProgramu_1" service_program="False" visible="True">
      <description>
      </description>
      <information_template>
      </information_template>
      <threads>
        <thread name="zakladka_1">          
        </thread>
      </threads>
    </program>

    <program name="NazwaProgramu_2" service_program="False" visible="True">
      <description>
      </description>
      <information_template>
      </information_template>
      <threads>
        <thread name="zakladka_1">          
        </thread>
        <thread name="zakladka_2">          
        </thread>
      </threads>
    </program> 

    (...)

  </programs>
</device>

0

massther skoro już mnie wcześniej naprowadzałeś, prosiłbym żebyś jeszcze raz zerknął na całość mojej klasy:
widze, że działa ale rzuć okiem czy nadal mam tam jakieś błędy i przede wszystkim czy jest to zgodnie z tym jak to sie powinno robić.

 
        class PlikDXML
        {  
        private string sciezkaPliku;
        private int iloscWezlow;
        private string nazwaMaszyny; 
        private string[] tablicaNazwWezlowProgram;
        private XmlDocument Doc;

        public PlikDXML() //konstruktor 
        {
            sciezkaPliku = "";
            iloscWezlow = 0;
            nazwaMaszyny = "brak";
            Doc = new XmlDocument();
        }
        public void ZaladowaniePliku(string path) //OK
        {
            sciezkaPliku = path;
            try
            {
                Doc.Load(sciezkaPliku);
            }
            catch (Exception)
            {
                
            } 
            this.IloscWezlow = Doc.GetElementsByTagName("program").Count;
            this.NazwaMaszyny = Doc.GetElementsByTagName("device").Item(0).Attributes["name"].Value.ToString();
            this.WczytanieNazwWezlow(); 
        }
        private void WczytanieNazwWezlow() //OK
        {
            tablicaNazwWezlowProgram = new string[this.IloscWezlow];
            for (int i = 0; i < this.IloscWezlow; i++)
            {
                tablicaNazwWezlowProgram[i] = Doc.GetElementsByTagName("program").Item(i).Attributes["name"].Value;
            }
        }
        public int IloscWezlow  //wlasciwosc OK
        {
            get
            {
                return iloscWezlow;
            }
            set
            {
                iloscWezlow = value;
            }
        }
        public string NazwaMaszyny   //walsciwosc OK
        {
            get
            {
                return nazwaMaszyny;
            }
            set
            {
                nazwaMaszyny = value;
            }
        }
        public string[] TablicaNazwWezlowProgram  //wlasciwosc  OK
        {
            get
            {
                return tablicaNazwWezlowProgram;
            }  
        }      
        public void ZapiszPlik()
        {
            try
            {
                Doc.Save(sciezkaPliku);
            }
            catch (Exception)
            {             
                
            }            
        }
        public XmlNode KopiujProgram(string zaznaczony) //OK
        {            
            XmlNode kopiaProgramu = null;
            kopiaProgramu = Doc.SelectSingleNode("/device/programs/program[@name='" + zaznaczony + "']");
            //do kopiaProgramu łąduje wezel z Doc wyszukany po nazwie programu (po atrybucie 'name')    
            return kopiaProgramu;
        }
        public void WstawProgram(XmlNode program, string zaznaczony) // OK
        {
            XmlElement elem = Doc.CreateElement("program");
            elem.InnerXml = program.InnerXml;
           
            XmlAttribute programAtrybut1 = Doc.CreateAttribute("name");
            XmlAttribute programAtrybut2 = Doc.CreateAttribute("service_program");
            XmlAttribute programAtrybut3 = Doc.CreateAttribute("visible");
            programAtrybut1.Value = zaznaczony;
            programAtrybut2.Value = "False";
            programAtrybut3.Value = "True";
            elem.Attributes.Append(programAtrybut1);
            elem.Attributes.Append(programAtrybut2);
            elem.Attributes.Append(programAtrybut3);
            
                XmlNode ostatniWezelProgram = null;
                XmlNode wezelNadrzednyPrograms;
                wezelNadrzednyPrograms = Doc.SelectSingleNode("/device/programs");
                ostatniWezelProgram = wezelNadrzednyPrograms.LastChild;                
                wezelNadrzednyPrograms.InsertAfter(elem, ostatniWezelProgram);
            
            this.IloscWezlow = Doc.GetElementsByTagName("program").Count;
            this.WczytanieNazwWezlow();
        }
        public void UsunProgram(string zaznaczony) //OK
        {            
            XmlNode zaznaczonyWezel = null;                            
            zaznaczonyWezel = Doc.SelectSingleNode("/device/programs/program[@name='" + zaznaczony + "']");
            zaznaczonyWezel.ParentNode.RemoveChild(zaznaczonyWezel);     //usuwam wezel wraz ze znacznikami <program>...</program>
            this.IloscWezlow = Doc.GetElementsByTagName("program").Count;
            this.WczytanieNazwWezlow();
        }
    }
1

Konstruktor.
Nie żeby ustawianie tam tych wartości domyślnych (dla ciebie domyślnych) było błędem, ale wydaje mi się zbyteczne. Ja zainicjalizowałbym pola. Wydaje mi się to bardziej przejrzyste oraz łatwiej dodać kolejny konstruktor. sciezkaPliku, nazwaMaszyny zostawił bym jako null, to gui używające tego obiektu powinno umieć zinterpretować tą informację jak "brak". Także tworzenie nowego obiektu typu XmlDocument (pole Doc) uważam za zbędne, bo ten obiekt ma sens istnienia w twojej klasie dopiero jeśli wczytamy jakiś plik.

Właściwości.
Ani w getterach, ani w setterach nie przeprowadzasz żadnych dodatkowych operacji przed zwróceniem wartości, więc z uwagi na czytelność kodu użyłbym automatycznych właściwości (jeśli to się jakoś inaczej nazywa niech ktoś mnie poprawi, bo nie pamiętam), czyli:

public int IloscWezlow { get; set; }
public string NazwaMaszyny { get; set; }
// etc.

Nie potrzebujesz wtedy pól prywatnych. Mniej kodu robiącego to samo - łatwiejsze czytanie klasy.

Ten try-catch w ZaladowaniePliku jest bez sensu, bo jak poleci ci exception przy Load'owaniu pliku, to się nawet o tym nie dowiesz. A dalsza linijka w tej metodzie Doc.GetElementsByTagName("device").Item(0)... i tak spowoduje że wyskoczy ci exception, bo nie będzie Itemu o indexie zero. Podobna uwaga do zapisu, łapanie exception bez reakcji na niego. Czasem się tak robi, ale w tej sytuacji to chyba jest niewłaściwe.

W UsunProgram pobierasz węzeł na podstawie nazwy do zmiennej zaznaczonyWezel, a następnie wywołujesz na nim metodę, bez sprawdzenia czy taki węzeł istnieje (czyli czy zaznaczonyWezel != null). Wiem że twoja klasa działa na podstawie gui, ale takie klasy pisze się tak, aby funkcjonowały także w oderwaniu od gui, czyli jako parametr tej metody może ktoś kiedyś podać taką wartość, której nie będzie w twoim xml'u.

Nie wiem jak używasz metody KopiujProgram, ale wydaje mi się że powinieneś zrobić kopię tego węzła, który zwracasz, bo ten który odnalazłeś i zwracasz jest już osadzony w jakimś xml. Każda jego modyfikacja zmienia oryginalny węzeł.

0

poprawiłem właściwości i konstruktor (ograniczyłem go do zainicjownia sciezki dostępu z wartoscia null)

        private string sciezkaPliku;
        private string[] tablicaNazwWezlowProgram;
        private XmlDocument Doc;
        bool poprawnoscWczytaniaPliku;
        bool poprawnoscZapisaniaPliku;

        public int IloscWezlow { get; set; }//wlasciwosc automatyczna OK    
        public string NazwaMaszyny { get; set; }//walsciwosc automatyczna OK
     
        public PlikDXML() //konstruktor 
        {
            sciezkaPliku = null;            
        }
        public void ZaladowaniePliku(string path) //OK
        {            
            sciezkaPliku = path;
            try
            {
                Doc = new XmlDocument();
                Doc.Load(sciezkaPliku);                
                poprawnoscWczytaniaPliku = true;
            }
            catch (Exception)
            {
                poprawnoscWczytaniaPliku = false;
            }

            if (poprawnoscWczytaniaPliku)
            {
                this.IloscWezlow = Doc.GetElementsByTagName("program").Count;
                this.NazwaMaszyny = Doc.GetElementsByTagName("device").Item(0).Attributes["name"].Value.ToString();
                this.WczytanieNazwWezlow();
            }         
        }        

z pola "poprawnoscWczytaniaPliku" dalej mam korzystać w każdej metodzie klasy która ma do czynienia z Doc, przez zwykłego if'a tak jak poniżej?

        private void WczytanieNazwWezlow() //OK
        {
            if (poprawnoscWczytaniaPliku)
            {
                tablicaNazwWezlowProgram = new string[this.IloscWezlow];
                for (int i = 0; i < this.IloscWezlow; i++)
                {
                    tablicaNazwWezlowProgram[i] = Doc.GetElementsByTagName("program").Item(i).Attributes["name"].Value;
                }  
            }            
        } 

Nie wiem co sie zwykle robi w przypadku exceptiona przy zapisywaniu do pliku, czy moze byc tak?

         public void ZapiszPlik()
        {            
            try
            {
                Doc.Save(sciezkaPliku);
                poprawnoscZapisaniaPliku = true;
            }
            catch (Exception)
            {
                poprawnoscZapisaniaPliku = false;
            }                 
        } 

z pola (lub właściwosci automatycznej) "poprawnoscZapisaniaPliku" moge dalej w Form1 skorzystac i wyswietlic komunikat w MessageBox ze nie zapisalo mi do pliku i nalezy kliknąc zapisz jeszcze raz? Cos takiego?

Z KopiujProgram() korzystam tak ze wstawiam je bezpośrednio jako argument formalny do WstawProgram(),

 
        private void lewo_prawo_Click(object sender, EventArgs e)
        {     
            //warstwa wizualizacji
            if (ZgodnoscMaszyn())
            {
                if (!(listBox_lewy.SelectedItem == null))
                {
                    if (!listBox_prawy.Items.Contains(listBox_lewy.SelectedItem) && zaladowanieLewego)
                    {
                        prawyPlik.WstawProgram(lewyPlik.KopiujProgram(listBox_lewy.SelectedItem.ToString()), listBox_lewy.SelectedItem.ToString());
                        if (radioButton_Przenieś.Checked == true)
                        {
                            lewyPlik.UsunProgram(listBox_lewy.SelectedItem.ToString());
                            listBox_lewy.SelectedItems.Clear();
                            AktualizacjaLewegoPanelu();
                        }
                        AktualizacjaPrawegoPanelu();                        
                    }
                    else
                    {
                        MessageBox.Show("Program " + listBox_lewy.SelectedItem.ToString() + "\njuż istnieje w docelowej lokalizacji.", "Informacja", MessageBoxButtons.OK, MessageBoxIcon.Information);
                    }
                }
                else
                {
                    MessageBox.Show("Nic nie zaznaczono");  
                }
            }
            else
            {   
                MessageBox.Show("Niezgodność maszyn!");
            }
            //koniec warstwy wizualizacji

        }//działa - OK

W ksiazce wyczytalem ze nie powinno sie zagniezdzac wiecej jak 3 if'y ale nie wiem jak przecedzic te wszsytkie warunki w inny sposob (jako kolejne else if{}, jakos mi nie pasuja). Co do kopiowania/klonowania wezla wczytanego prze KopujProgram() to nie pomyslalem o tym, bo i nie planowalem modyfikowac zadych wezlow tym programem tylko je przesuwac, usuwac, wstawiac. Ale sprobuje to zrobic.

metode UsunProgram() zmienilem na taka, czy to o to chodzi? przeliczenie wezlow i pobieranie nazw wezlow tez robie tylko wtedy gdy skasuje wezel, jak mi nie skasuje to nic sie nie zmienia.

        public void UsunProgram(string zaznaczony) //OK
        {
            if (poprawnoscWczytaniaPliku)
            {
                XmlNode zaznaczonyWezel = null;
                zaznaczonyWezel = Doc.SelectSingleNode("/device/programs/program[@name='" + zaznaczony + "']");
                if (zaznaczonyWezel != null)
                {
                    zaznaczonyWezel.ParentNode.RemoveChild(zaznaczonyWezel); //usuwam wezel wraz ze znacznikami <program>...</program>
                    this.IloscWezlow = Doc.GetElementsByTagName("program").Count;
                    this.WczytanieNazwWezlow(); 
                }
            }
        } 

mam w klasie pole "private string[] tablicaNazwWezlowProgram" jak z tego zrobic wlasciwosc? mam problem z ustawieniem set'a do tablicy której rozmiar sie zmienia.

1

W twoim przypadku możesz mieć inicjalizację przy deklaracji pól:

private string sciezkaPliku = null;
private string[] tablicaNazwWezlowProgram = null;
private XmlDocument Doc = null;

wtedy konstruktora możesz w ogóle nie pisać, albo będzie on pusty. Ale tak jak masz to nie błąd, to co proponuję to kosmetyka, która jest powszechnie stosowane, bo zwiększa czytelność, kiedy od razu widzisz przy deklaracji wartość domyślną pola.

Można założyć że jeśli plik wczytał się błędnie to doc jest (ciągle) null. Więc dodatkowa zmienna nie jest potrzebna, wystarczy sprawdzać na początku każdej metody czy doc jest null i jeśli tak to zakończyć metodę, czy rzucić wyjątkiem, mówiącym że plik nie został wczytany.
Tak jak użyłeś poprawnoscWczytaniaPliku jest poprawnie, a moja powyższa sugestia to znowu kosmetyka, która eliminuje ci z kodu dodatkową zmienną, a daje niemal tą samą funkcjonalność.

Odczyt/Zapis i try-catch
Jeśli jak proponowałem doc = null będzie informacją że plik nie został wczytany (jeszcze czy w wyniku błędu) to w catch przy Load powinieneś ustawiać doc na null, bo wcześniej przed load musiałeś utworzyć obiekt XmlDocument. Ale powinieneś także wyrzucić ten exception wyżej, aby gui, czy inna klasa używająca twojej, potrafiła jakoś odpowiednio sama zareagować na błąd wczytania pliku. Do tego zostały pomyślane exceptiony.
Podobnie przy zapisie, tam try-catch nie jest potrzebny, bo twoja klasa na ten błąd w żaden sposób nie reaguje. To np. gui ma wyświetlić komunikat: "Błąd zapisu", ale musi mieć szansę dowiedzieć się o tym. Jeśli nie chcesz pozwolić żeby exception wylatywał sensowniejszym jest zwracanie wartości bool odpowiadającej na pytanie czy zapis się powiódł, tylko w przypadku błędu dowiesz się w klasie używającej twojej, że zapis nie udał się, ale bez informacji dlaczego, taką informację zawiera exception, który skutecznie przechwyciłeś.
Jeśli będziesz łapał i re-wyrzucał exception wyżej, to zrób to tak:

 try {...}
catch(Exception ex)
{
  // coś robię
  throw;
}

a nie tak!!!

try {...}
catch(Exception ex)
{
  // coś robię
  throw ex;
}

Dlaczego? W drugim przypadku cały stack trace zostanie wyczyszczony i będzie zaczynał się od miejsca z którego ponownie wyrzuciłeś exception. Więc w ten sposób tracisz dość istotne informacje, które po zalogowaniu błędu mogą posłużyć do analizy błędu.

"W ksiazce wyczytalem ze nie powinno sie zagniezdzac wiecej jak 3 if'y" - zapewne chodziło o czytelność kodu, bo technicznie jest to możliwe i w twoim przypadku uzasadnione. Można ewentualnie podzielić ten kod na jakieś metody, dzięki czemu zyskasz większą przejrzystość. Chociaż w tym przypadku nie uważam tego za konieczne.
if (!(listBox_lewy.SelectedItem == null)) można pomyśleć nad wyelminowaniem tego if przez wyszarzanie odpowiedniego przycisku do przenoszenia, jeśli na danej liście, nic nie jest zaznaczone

hmmm użycie tego Wstaw-Kopiju nie do końca mi się podoba, ale muszę dokładniej się temu przyjrzeć. Ale spadam za chwilę do domu, więc wieczorem coś o tym napiszę.

0

przyznam się, że w try/catch na razie orientuje się najmniej, na razie skupiłem się na tym aby w końcu program zadziałał, jak zadziałał (to mój drugi program w C# nie licząc "Hello World" ;) ) to moge w koncu zacząc wnikać w try/catch i dopracowywać całość.

Jedno w czym na razie widze sprzeczność to wyciąganie wyjątków z metod klasy1(moja klasa PlikDXML) do klasy2 (Form1). Pobierznie w literaturze omawiane są aplikacje consolowe i tam po prostu w sekcji catch stoi:
Console.WritleLine("wyjątek " + ex + " cos tam"); - ewentualnie cos podobnego

W aplikacji okienkowej bez sensu wstawiać MessageBox w klasie jak u mnie np. PlikDXML (mieszanie logiki i wizualizacji tak?), czyli jak przekazac informacje o wyjątku wystepującym w mojej klasie do klasy Form1? Nowe pola? metody dostepu do wartości tych pół i obsługa z tym związana w Form1? Czy może właśnie obiekt (Exception to obiekt?) "ex" mozna zrobic takim polem?

Widze ze cos jest nie tak po uruchomieniu swojego programu bo nie ladujac żadnego pliku z programami klikam zapisz, i nic sie nie dzieje, metoda ZapiszPlik() sie nie wykonuje, if nie pozwala:

        public void ZapiszPlik()
        {
            if (poprawnoscWczytaniaPliku)
            {
                try
                {
                    Doc.Save(sciezkaPliku);
                    poprawnoscZapisaniaPliku = true;
                }
                catch (Exception ex)
                {
                    poprawnoscZapisaniaPliku = false;
                    throw; 
                }                
            }
        } 

w zdarzeniu kliniecia przycisku, wykonuje sie sekcja try chociaz po metodzie wyzej widac ze ona nic nie robi:

 
private void btn_zapiszL_Click(object sender, EventArgs e)
        {
            try
            {
                lewyPlik.ZapiszPlik();
            }
            catch (Exception ex)
            {
                MessageBox.Show("Szczegóły błędu\n\n" +ex +"", "Błąd!", MessageBoxButtons.OK, MessageBoxIcon.Error);
            }
        }//działa - OK

Nie wiem w jakim przypadku zobaczę komunikat z sekcji catch po kliknieciu przycisku zapisz, brakuje mi wiedzy co musi sie wydarzyc aby zobaczyc ten komunikat.

Próbowałem uruchomic swoj program, zaladowac plik z programami, w windowsie usunac plik ktory ladowalem i wtedy w programie kliknac "zapisz" ale po prostu utworzylo mi program o takiej samej nazwie i takiej samej zawartosci (w sciezcePliku przetrzymuje nazwe pliku).
folderów juz windows nie pozwala mi ruszyc, ani zmienic nazwy ani usunac.

Czy metoda WstawProgram() jest prawidłowo zrobiona? czy można to szybciej i latwiej zrobic bez tworzenia elementu?
Pytam bo

elem.InnerXml = program.InnerXml;

ten sposób praktycznie znalazłem po omacku dzięki czerwonej kropie przy debugowaniu, szukałem co gdzie jest i potem przypisywałem wartość, dlatego zastanawiam sie czy to na pewno tak powinno byc.

 
        public void WstawProgram(XmlNode program) // OK
        {
            if (poprawnoscWczytaniaPliku)
            {
                XmlElement elem = Doc.CreateElement("program");
                elem.InnerXml = program.InnerXml;

                XmlAttribute programAtrybut1 = Doc.CreateAttribute("name");
                XmlAttribute programAtrybut2 = Doc.CreateAttribute("service_program");
                XmlAttribute programAtrybut3 = Doc.CreateAttribute("visible");
                programAtrybut1.Value = program.Attributes["name"].Value;
                programAtrybut2.Value = program.Attributes["service_program"].Value;
                programAtrybut3.Value = program.Attributes["visible"].Value;
                elem.Attributes.Append(programAtrybut1);
                elem.Attributes.Append(programAtrybut2);
                elem.Attributes.Append(programAtrybut3);

                XmlNode ostatniWezelProgram = null;
                XmlNode wezelNadrzednyPrograms = null;
                wezelNadrzednyPrograms = Doc.SelectSingleNode("/device/programs");
                ostatniWezelProgram = wezelNadrzednyPrograms.LastChild;
                wezelNadrzednyPrograms.InsertAfter(elem, ostatniWezelProgram);

                this.IloscWezlow = Doc.GetElementsByTagName("program").Count;
                this.WczytanieNazwWezlow();
            }            
        }

Metoda KopiujProgram(), czy dodanie Clone() rozwieje wątpliwości co do działania tej metody?

kopiaProgramu = Doc.SelectSingleNode("/device/programs/program[@name='" + zaznaczony + "']").Clone(); 

jeżeli dobrze zrozumiałem Twoje wątpliwości to:

kopiaProgramu = Doc.SelectSingleNode("/device/programs/program[@name='" + zaznaczony + "']"); 

ładowało do kopiaProgramu referencje do konkretnego węzła w Doc, i ogólnie cały obiekt "kopiaProgramu" pozostawał "w Doc". teraz w przypadku przenoszenia z jednego pliku do drugiego jednego wezla, tworzyłem dwa pliki "zrośnięte jednym węzłem"?

jak zastosuje Clone() to do obiektu kopiaProgramu() wstawie kopie wezła z Doc z nowymi referencjami, niezależnymi od Doc czyli kopiaProgramu bedzie juz poza Doc? Czyli pozniej w przypadku przenoszenia z jednego pliku do drugiego pojedynczego wezla, przenosze ten wezel a "takim jakby buforze" czyli niezaleznie zapisuje dwa pliki nie "zrosniete jednym wezlem"?

Dobrze rozumuje?

0

Nareszcie udalo mi sie wywolac wyjatek :) Dla porządku i chyba już końca moich zmagań z tym programem załączam sposób wywołania tego błędu.

1)Po prostu otworzyłem plik swoim programem z pendriva,
2)program został załadowany do Doc,
3)nastepnie wyjałem pendrive z kompa,
4)i kliknąłem Zapisz w swoim programie.

metoda zapisu:

 
        public void ZapiszPlik()
        {
            if (poprawnoscWczytaniaPliku)
            {
                try
                {
                    Doc.Save(sciezkaPliku);
                    poprawnoscZapisaniaPliku = true;
                }
                catch (Exception ex)
                {
                    poprawnoscZapisaniaPliku = false;
                    throw; 
                }                
            }
        }

wywolawanie metody w Form1:

private void btn_zapiszL_Click(object sender, EventArgs e)
        {
            if (zaladowanieLewego)
            {
                try
                {
                    lewyPlik.ZapiszPlik();
                }
                catch (Exception ex)
                {
                    MessageBox.Show("Błąd zapisu do pliku\n\n" + ex + "", "Błąd!", MessageBoxButtons.OK, MessageBoxIcon.Error);
                }
            }
            else
            {
                MessageBox.Show("Nie masz co zapisywać! \nŻaden plik nie został otwarty!", "Błąd!", MessageBoxButtons.OK, MessageBoxIcon.Error);
            }
            
        }//działa - OK 

a to efekt:
user image

po klinieciu ok, okno się zamyka i program dziala dalej :)

Wiec chyba o to chodzi zeby teraz np. przechwycić taki exception, i powiadomic uzytkownika ze próba zapisu nie powiodła się bo nie ma dostepu do pliku który miał być zapisany?

0

Brawo, dokładnie tak. Typ exception lub jakieś dodatkowe jego parametry mówią o dokładnej przyczynie. Więc jeśli spodziewasz się w jakimś miejscu określonego typu wyjątku, to powinieneś mieć catch z takim typem, oczywiście catch można mieć kilka z różnymi typami wyjątków. Pamiętaj tylko że łapany jest pierwszy pasujący może być wyższego poziomu (w sensie hierarchii dziedziczenia), więc zasadą jest że łapie się najpierw najbardziej szczegółowe a na końcu najbardziej ogólne wyjątki.

przykład:

try
{
  Zapiszplik(path);
}
catch(DirectoryNotFoundException direx)
{/* brak katalogu */}
catch(DriveNotFoundException driveex)
{/* brak dysku */}
catch(PathTooLongException pathex)
{/* ścieżka zbyt długa */}
catch(IOException ioex)
{/* inny błąd związany z zapisem */}
catch(Exception ex)
{/* inny dowolny, nieznany błąd */}

DirectoryNotFoundException, DriveNotFoundException, PathTooLongException wszystkie dziedziczą z IOException, dlatego zostały umieszczone przed nim. Bo inaczej zostałby złapany IOException i wykonana obsługa dla tego typu wyjątku, kolejne bloki catch już by się nie wykonały.

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