Closed Bug 1206174 Opened 6 years ago Closed 6 years ago

Improve code readability of FMRadioService

Categories

(Firefox OS Graveyard :: Gaia::FMRadio, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(firefox44 fixed)

RESOLVED FIXED
FxOS-S8 (02Oct)
Tracking Status
firefox44 --- fixed

People

(Reporter: sotaro, Assigned: sotaro)

Details

Attachments

(2 files, 1 obsolete file)

Recently, I checked FMRadioService code. It seems better to improve code readability. For example, it is not easy to understand which function is hal function.
Assignee: nobody → sotaro.ikeda.g
Attached patch patch part 2 - Use lambda (obsolete) — Splinter Review
Attachment #8663095 - Attachment description: patch - Remove implicit mozilla::hal namespace usage → patch Part 1 - Remove implicit mozilla::hal namespace usage
Attachment #8663096 - Attachment description: patch - Use lambda → patch part 2 - Use lambda
Attachment #8663095 - Flags: review?(alwu)
Attachment #8663096 - Flags: review?(alwu)
Attachment #8663095 - Flags: review?(alwu) → review+
Comment on attachment 8663096 [details] [diff] [review]
patch part 2 - Use lambda

Review of attachment 8663096 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/fmradio/FMRadioService.cpp
@@ +752,5 @@
> +  NS_DispatchToMainThread(NS_NewRunnableFunction(
> +    [self] () -> void {
> +      self->NotifyFMRadioEvent(RDSEnabledChanged);
> +    }
> +  ));

This codes is called lots of times.
Maybe we can use the wrapper function, like DispatchRMRadioEventToMainThread(event).

@@ +776,5 @@
> +    NS_DispatchToMainThread(NS_NewRunnableFunction(
> +      [self] () -> void {
> +        self->NotifyFMRadioEvent(RDSEnabledChanged);
> +      }
> +    ));

Ditto.

@@ +991,5 @@
> +    NS_DispatchToMainThread(NS_NewRunnableFunction(
> +      [self] () -> void {
> +        self->NotifyFMRadioEvent(PIChanged);
> +      }
> +    ));

Ditto.

@@ +1006,5 @@
> +    NS_DispatchToMainThread(NS_NewRunnableFunction(
> +      [self] () -> void {
> +        self->NotifyFMRadioEvent(PTYChanged);
> +      }
> +    ));

Ditto.

@@ +1043,5 @@
> +        NS_DispatchToMainThread(NS_NewRunnableFunction(
> +          [self] () -> void {
> +            self->NotifyFMRadioEvent(PSChanged);
> +          }
> +        ));

Ditto.

@@ +1063,5 @@
> +        NS_DispatchToMainThread(NS_NewRunnableFunction(
> +          [self] () -> void {
> +            self->NotifyFMRadioEvent(RadiotextChanged);
> +          }
> +        ));

Ditto.

@@ +1109,5 @@
> +      NS_DispatchToMainThread(NS_NewRunnableFunction(
> +        [self] () -> void {
> +          self->NotifyFMRadioEvent(RadiotextChanged);
> +        }
> +      ));

Ditto.

@@ +1128,5 @@
> +        NS_DispatchToMainThread(NS_NewRunnableFunction(
> +          [self] () -> void {
> +            self->NotifyFMRadioEvent(RadiotextChanged);
> +          }
> +        ));

Ditto.

@@ +1169,5 @@
> +      NS_DispatchToMainThread(NS_NewRunnableFunction(
> +        [self] () -> void {
> +          self->NotifyFMRadioEvent(RadiotextChanged);
> +        }
> +      ));

Ditto.

@@ +1185,5 @@
> +      NS_DispatchToMainThread(NS_NewRunnableFunction(
> +        [self] () -> void {
> +          self->NotifyFMRadioEvent(PTYChanged);
> +        }
> +      ));

Ditto.

@@ +1213,5 @@
> +  NS_DispatchToMainThread(NS_NewRunnableFunction(
> +    [self] () -> void {
> +      self->NotifyFMRadioEvent(NewRDSGroup);
> +    }
> +  ));

Ditto.
Attachment #8663096 - Flags: review?(alwu) → review+
(In reply to Alastor Wu [:alwu] from comment #3)
> Comment on attachment 8663096 [details] [diff] [review]
> patch part 2 - Use lambda
> 
> Review of attachment 8663096 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/fmradio/FMRadioService.cpp
> @@ +752,5 @@
> > +  NS_DispatchToMainThread(NS_NewRunnableFunction(
> > +    [self] () -> void {
> > +      self->NotifyFMRadioEvent(RDSEnabledChanged);
> > +    }
> > +  ));
> 
> This codes is called lots of times.
> Maybe we can use the wrapper function, like
> DispatchRMRadioEventToMainThread(event).
> 

Yea, I'll update it.
Attachment #8663096 - Attachment is obsolete: true
Attachment #8663703 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/bd1f2144cd84
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S8 (02Oct)
You need to log in before you can comment on or make changes to this bug.