"Troels Thomsen" <nej tak ...> wrote in message
news:474f66be$0$15898$edfadb0f@dtext01.news.tele.dk...
>
> Hej,
>
> Jeg sidder og prøver nedenstående eksempel fra msp-gcc pakken af.
> Det er et observerpattern der laver en night-rider effekt på nogle LED's
> på en embedded processor demo board.
>
> Lige nu synes jeg næsten at eksemplet har flere problemer end fordele, men
> ville lige høre jeres uforbeholdne mening om designet før jeg opgiver det
> helt.
>
> Det er noget af en omgang ....
> ... here goes :
>
>
> /*
> This examples shows how to to something simple in a complex fashion.
> It's point was to use C++ features to test the msp430-g++ compiler.
> */
>
> #include "hardware.h"
>
> class Subject;
> class Observer;
>
> typedef class Observer *ObserverPtr;
>
> // ----- fixed length list
> class Tuple {
> public:
> unsigned int length;
Den kan vist lige så godt være const
const unsigned int length;
> ObserverPtr * elements;
>
> Tuple(ObserverPtr * elements, unsigned int length) :
> length(length), elements(elements)
> {
> }
> };
>
> // ----- observer pattern base clases
>
> /**
> viewer
> */
> class Observer {
> public:
> virtual void update(Subject *subject);
Skal den ikke være pure virtual, og eventuelt const
virtual void update(Subject* subject) const = 0;
> };
>
> /**
> data model
> */
> class Subject {
> private:
> Tuple *observers;
> public:
> Subject(Tuple *observers) :
> observers(observers)
> {
> }
>
> unsigned short value;
Et public datamedlem ?
Det ligner noget snavs.
Og det bliver ikke bedre af at der konkrete data i en basisklasse, der
indgår i et framework.
>
> virtual void notify() {
Er det virkeligt rigtigt at den skal være virtuel ?
> Tuple *obslist = observers;
Den er overflødig.
> Observer *observer;
Flyt denne erklæring ind i løkken, så den bliver initialiseret samtidig.
> for (unsigned int i = 0; i<observers->length; i++)
Generelt foretrækker jeg stilen
for(...;i != x; ++i)
fordi:
i != x fejler mere åbenlyst, _hvis_ der er noget galt med programmet
i != x generelt stiller mindre krav til typen af i og x. Det spiller en
rolle i generisk programmering
++i er muligvis mere effektiv end i++, fordi den returnerer den nye
værdi. Det spiller en rolle i generisk programmering
og der er aldrig nogen ulemper ved den form.
> {
> observer = (Observer *)(obslist->elements)[i];
Det cast er overflødigt.
> if (observer) {
Kan man overhovedet forestille sig at den kan være 0 ?
Skift den ud med en assert
> observer->update(this);
> }
> }
> }
> };
>
>
> // ----- knight-rider like LED play
>
> /**
> viewer implementation: displaying value on port (LED araray)
> */
> class LedDisplay: public Observer {
> private:
> void *port;
Hvorfor ikke have typesikkerhed ?
F.eks. med templates.
> public:
> LedDisplay(void *port) :
> Observer(), port(port)
> {
> }
>
> void update(Subject *subject) {
> ((struct port_full_t *)(port))->outport = subject->value;
Hvorfor ikke bruge klarere C++ type cast, som reinterpret_cast ?
Og bedre helt undgå det ved at have typesikkerhed
> }
> };
>
> /**
> alternative viewer implementation: displaying inverted value
> on port (LED araray)
> */
> class InverseLedDisplay: public Observer {
> private:
> void *port;
Samme her.
> public:
> InverseLedDisplay(void *port) :
> Observer(), port(port)
> {
> }
>
> void update(Subject *subject) {
> ((struct port_full_t *)(port))->outport = ~subject->value;
Og samme her.
> }
> };
>
> /**
> data model: shift around bits, notify Observer on each change
> */
> class SimpleShifter: public Subject {
> private:
> unsigned short count;
> public:
> SimpleShifter(Tuple *observer) :
> Subject(observer)
> {
> count = 0;
> }
>
> void next() {
> if ((count & 0xf) < 8) {
> value = (1<<(count&7));
> } else {
> value = (0x80>>(count&7));
> }
> count++;
> notify();
> }
> };
>
> /**
> data model: shift around bits, notify Observer on each change
> */
> class Shifter: public Subject {
> private:
> unsigned short count;
> public:
> Shifter(Tuple *observer) :
> Subject(observer)
> {
> count = 0;
Hvorfor pludselig bruge assignment i stedet for initialisering.
> }
>
> void next() {
> value = (1<<(count&7)) | (0x80>>(count&7));
> count++;
> notify();
> }
> };
>
> /**
> Delay function.
> */
> void delay(unsigned int d) {
> unsigned int i;
> for (i = 0; i<d; i++) {
> nop();
> nop();
> }
> }
>
> /**
> Main function with some blinking leds
> */
> int main(void) {
> <cut>
> LedDisplay disp1(&port1);
> InverseLedDisplay disp2(&port2);
> ObserverPtr obs2[2] = {&disp1, &disp2};
Eksplicit angivelse af størrelse er overflødig
> Tuple obslist2(obs2, 2);
Lad compileren beregne størrelsen
Tuple obslist2(obs, sizeof(obs2)/sizeof(obs2[0]));
> SimpleShifter shifter2(&obslist2);
>
> while (1) { //main loop, never ends...
> shifter2.next();
> delay(0x4fff);
> }
> }
>
>
>
> Kommentarer :
>
> 1)
> I embedded kode er man tit intereseret i at så meget som muligt er const,
> idet det så bliver placeret i ROM og ikke i RAM.
Generelt er der ikke nogen _garantier_ for hvad der kommer i ROM - det er et
spørgsmål om kvaliteten af implementeringen (compiler, linker etc).
I den rapport jeg linker nederst til kan man se nogle anbefalinger om det.
Generelt gælder det at jo mere man kan låse på compile-time.
Templates er en ualmindelig god måde at gøre det - bl.a. derfor er EC++
håbløs.
I dit eksempel bliver der brugt run-time dynamik (virtuelle metoder) til
noget der ikke er run-time dynamisk (hvilke dioder eller porte findes). Det
er senest bestemt på compile tidspunkt.
Hvis den viden ikke udnyttes får man et performance overhead.
Så en analyse af hvad der er run-time dynamisk og run-time statisk siger:
* dynamisk:
* værdien der skal vises
* statisk:
* hvilke porte der skal bruges til hvilket formål
Resten af designet er et spørgsmål om kobling i systemet.
Observer design bruges typisk for at lave afkobling mellem forskellige dele
af systemet.
> I eksemplet er der slet ikke gjort nok ud af at få det placeret i const
> memory. Det er vel til at gå til ... Der bliver brugt nogen-og-tyve bytes
> ram lige nu, og det skulle gerne ned på det, som klassemedlemmerne count,
> value, port fylder i sig selv.
Port behøver ikke at ligge i RAM.
Konstruktionen med virtuelle metoder, gør at hver klasse (typisk) indeholder
en peger til en virtuel funktion tabel (vtable).
>
> 2)
> Der er nogle casts til Observer* nogle steder hvor jeg tænker om ikke det
> skulle være template parametriseret?
> Der skal det måske siges at Embedded C++ har ingen templates og STL. Måske
> har forfatteren tænkt at han ville holde det i ren EC++. Min IAR compiler
> understøtter dog også Extended EC++, med templates, som jeg har prøvet, og
> en reduceret STL som jeg ikke har prøvet. Så længe det kan ordnes af
> compile-time, så har jeg ingen indvendinger mod at aktivere EEC++.
Jeps - statisk typecheck er at foretrække i et statisk typet sprog som C++
>
> 3)
> class Subject indeholder både selve vores "data", og så en "stor" notify
> funktion. Notify er vel ens for alle de gange jeg skal bruge dette
> pattern, så skulle den ikke ligge i en base klasse ? Lille problem.
Jeps - det er et sygdomstegn.
>
> 4)
> Observer har en metode virtual void update(Subject *subject);
> Det bliver jeg nødt til at lave pure virtual , = 0 , eller lave tom
> implementation {} for at IAR vil æde den. (Eksemplet er fra en gcc port
> til en anden embedded processor) .Ellers brokker den sig over manglende
> external _vtbl. Prøvede lige på visual studio, synes at den også brokkede
> sig (?).
> Er forskellen på {} og =0 i dette eksempel den, at i førstnævnte kommer
> der to implementationer af update() som den skal kunne holde styr på vha
> en v-table, og dermed får man runtime overhead ? I så fald foretrækker jeg
> den sidste !
Nej.
Der er ingen runtime overhead, men der er et plads overhead fordi du vil få
en tom funktion som aldrig bliver kaldt, men linkeren har svært ved at
gennemskue det og fjerne den. Det er fordi den tomme funktion bliver
refereret fra Observer's vtable der bliver brugt i forbindelse med
constructor-forløbet.
Kort sagt: det er bedst at lave den pure-virtual.
Både performance og design mæssigt.
>
> 5)
> Selve instantieringen er rimelig ugly imho. Føst objekterne, så array af
> objekter, så wrappes ind i den Tuple, så til sidst sammenkædningen af
> Subjekt og Observer.
Jeps
> På en PC ville man vel bruge noget i stil med
> SimpleShifter shifter2("new vector<Observer> (new
> LEDDisplay(&port1);)")
> New er ikke en mulighed. Jeg har ialt 2k ram, og de skal bruges til andre
> buffere, ikke til en heap.
Nej - nok ikke noget med new.
> Om Vector er en mulighed ved jeg ikke. Dels om den er i IARs STL lib, og
> dels om den kan ligge i flash udelukkende.
Generelt kan jeg varmt anbefale
http://www.open-std.org/jtc1/sc22/wg21/docs/TR18015.pdf
som indeholder en masse nyttig information om C++ og performance.
Den er skrevet af fantastisk dygtige folk .
Kig også på bogen
Multi-Paradigm DESIGN for C++
James O. Coplien
ISBN 0-201-82467-1
som beskriver en masse overvejelser omkring design med brug af flere
paradigmer end det objekt-orienteret, ud fra overvejelser om problem
domænets egenskaber, med hensyn til hvad der varierer og hvad der er
konstant.
Et bud på en alternativ løsning i C++ (ikke EC++) med brug af klasser og
templates:
#include <iostream>
#include <iomanip>
using namespace std;
class simple_shifter
{
public:
typedef unsigned char value_type;
simple_shifter() :
count_(0)
{}
unsigned char operator++()
{
const unsigned char result =
count_ < 8 ?
0x01 << count_ :
0x80 >> (count_ & 0x07);
count_ = (count_ + 1) & 0x0f;
return result;
}
private:
unsigned char count_;
private:
// make copy-constructor private and not implemented
simple_shifter(const simple_shifter&);
simple_shifter& operator=(const simple_shifter&);
};
template <const void* port>
class led_display
{
public:
void operator()(unsigned char value)
{ cout << port << ": " << setw(2) << setfill('0') << hex <<
static_cast<unsigned short>(value) << endl; }
};
template <const void* port>
class inverse_led_display
{
public:
void operator()(unsigned char value)
{ cout << port << ": " << setw(2) << setfill('0') << hex <<
static_cast<unsigned short>(~value & 0x0f) << endl; }
};
template <typename ShifterT, typename Disp1T, typename Disp2T>
class sequencer
{
public:
void operator()()
{
const typename ShifterT::value_type value = ++shifter_;
disp1_(value);
disp2_(value);
}
private:
ShifterT shifter_;
Disp1T disp1_;
Disp2T disp2_;
};
int main(void)
{
sequencer<
simple_shifter,
led_display<static_cast<void*>(0)>,
inverse_led_display<static_cast<void*>(0)> > seq;
while(1) {
seq();
}
}
Du kan selv lave funktionerne som skriver til cout om til noget passende for
dit problem.
Bemærk at der er ingen arv eller run-time polymofi.
Bemærk også at klasserne "led_display" og "inverse_led_display" har
fastkodet "porten" som en del af typen - det betyder at den ikke skal gemmes
i nogen variabel. Bemærk også at klasserne er helt uafhængige af
"sequencer" - i modsætning til Subject og Observer, der er gensidigt
afhængige i din kode.
Bemærk at klassen sequencer er uafhængig af den konkrete type "shifter",
"led_display" og "inverser_led_display".
Bemærk klassen "shifter" er uafhængig af "led_display",
"inverse_led_display" og "sequencer".
Uafhængighederne, i source koden til de enkelte klasser, gør at de kan
testes og bruges uafhængigt af hinanden.
Bemærk også at i "main" konfigureres det hele sammen baseret på typer i
stedet for variable og værdier, så compileren har mulighed for at lave en
performance mæssig meget optimal kode.
Compileren har muligvis lidt mere at lave end i din objektorienterede kode -
til gengæld får den embeddede processor mindre at lave, hvilket er en god
ting.
Hvis man laver de 2 display funktioner tomme, og oversætter programmet med
Visual C++ 2005 med Release mode bliver der lige præcis lagt een jump
instruktion ud og intet andet, for at implementere den uendelige løkke!
Det er simpelthen fordi koden giver compileren gode muligheder for at
optimere.
Hvis man måtte ønske at understøtte et variabelt antal "display" typer kan
det sagtens lade sig gøre med template specialisering, evt. med brug af
typelister.
Jeg håber også at det står lidt mere klart, hvorfor det er fundamentalt
forkert at EC++ ikke har templates med henvisning til performance i
embeddede systemer.
--
Venlig hilsen
Mogens Hansen