Prev: Polymorphism Downsides (was: what's the future of Object Oriented Programming)
Next: design problem
From: Leon Wu on 21 Sep 2006 23:24
If I post in wrong place. forgive me.
I hope some one can Refactor following code for me!
We have a system with 6 application. One application (Named it
ContrlPanel) is a control center, it get command from UI and order
other application do some work. All these application communicate with
each other through share memory. so now all job of ControlPanel is like
following.
1. poll the state of A.
2. according to the state of A order B to do something.
3. poll the state of C.
4. according to the state of C order E or F to do something.
because all this job with share memory need do again and again till
succeed.(try to lock it etc).
so we get some code like following
XXXXThread
{
while (!exit)
{
switch(state) {
case GETING_A_STATE:
if (GetAState()) {
state = DOING_SOMETHING_A;
}
break;
case DOING_SOMETHING_A:
if (DoSomeA()) {
state = GET_C_STATE;
}
break;
case GET_C_STATE:
....
}
}
}

From: H. S. Lahman on 22 Sep 2006 15:51
Responding to Wu...

> If I post in wrong place. forgive me.
> I hope some one can Refactor following code for me!
> We have a system with 6 application. One application (Named it
> ContrlPanel) is a control center, it get command from UI and order
> other application do some work. All these application communicate with
> each other through share memory. so now all job of ControlPanel is like
> following.
> 1. poll the state of A.
> 2. according to the state of A order B to do something.
> 3. poll the state of C.
> 4. according to the state of C order E or F to do something.
> because all this job with share memory need do again and again till
> succeed.(try to lock it etc).
> so we get some code like following
> XXXXThread
> {
> while (!exit)
> {
> switch(state) {
> case GETING_A_STATE:
> if (GetAState()) {
> state = DOING_SOMETHING_A;
> }
> break;
> case DOING_SOMETHING_A:
> if (DoSomeA()) {
> state = GET_C_STATE;
> }
> break;
> case GET_C_STATE:
> ....
> }
> }
> }

How does 'exit' get set? Is that related to why the loop was done this
way? (I assume it is just bound to a register that indicates whether
shared memory is locked.)

It seems to me this is just overcomplicating things. It is also using
'state' to describe two different kinds of conditions and that sort of
overloading is usually fragile. So I would try a simpler polling loop:

while (!exit)
{
if (GetAState())
DoSomeA()
if (GetCState())
DoSomeC()
...
}


*************
There is nothing wrong with me that could
not be cured by a capful of Drano.

H. S. Lahman
hsl(a)pathfindermda.com
Pathfinder Solutions
http://www.pathfindermda.com
blog: http://pathfinderpeople.blogs.com/hslahman
"Model-Based Translation: The Next Step in Agile Development". Email
info(a)pathfindermda.com for your copy.
Pathfinder is hiring:
http://www.pathfindermda.com/about_us/careers_pos3.php.
(888)OOA-PATH



From: Leon Wu on 24 Sep 2006 23:31
thanks for your reply.
In fact, above source code is just like goto statement, when you want
to do something, your assign the "state" variable to some value. the
"exit" flag will be set
after certain state is reached or the "quit" signal reach. this method
is really bad. Not only the code is like a mess, but sometimes the
modification of share memory will lost. I think the better solution
will be some kind of message(signal etc) instead of poll sharememory.
but now I only have rights to modify the control panel. so what I can
do is make this block of source code more readable.

From: H. S. Lahman on 26 Sep 2006 00:25
Responding to Wu...

> thanks for your reply.
> In fact, above source code is just like goto statement, when you want
> to do something, your assign the "state" variable to some value. the
> "exit" flag will be set
> after certain state is reached or the "quit" signal reach. this method
> is really bad. Not only the code is like a mess, but sometimes the
> modification of share memory will lost. I think the better solution
> will be some kind of message(signal etc) instead of poll sharememory.
> but now I only have rights to modify the control panel. so what I can
> do is make this block of source code more readable.

I assume one of the DoSomeX operations would set the exit flag. You
could deal with that without a global flag by substituting

while (TRUE)
{
if (GetAState())
if (!DoSomeA()) break;
if (GetCState())
if (!DoSomeC()) break;
...
}

in my proposal.

*************
There is nothing wrong with me that could
not be cured by a capful of Drano.

H. S. Lahman
hsl(a)pathfindermda.com
Pathfinder Solutions
http://www.pathfindermda.com
blog: http://pathfinderpeople.blogs.com/hslahman
"Model-Based Translation: The Next Step in Agile Development". Email
info(a)pathfindermda.com for your copy.
Pathfinder is hiring:
http://www.pathfindermda.com/about_us/careers_pos3.php.
(888)OOA-PATH



 | 
Pages: 1
Prev: Polymorphism Downsides (was: what's the future of Object Oriented Programming)
Next: design problem