After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 744461 - pulsesink: Enhance code readability in pulsesink_query
pulsesink: Enhance code readability in pulsesink_query
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.4.5
Other Linux
: Normal enhancement
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-02-13 11:14 UTC by Jimmy Ohn
Modified: 2015-02-13 22:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
pulsesink-Enhance-code-readability-in-pulsesink_query (1.26 KB, patch)
2015-02-13 11:19 UTC, Jimmy Ohn
none Details | Review
pulsesink-Enhance-code-readability-in-pulsesink_query (1.30 KB, patch)
2015-02-13 11:57 UTC, Jimmy Ohn
committed Details | Review

Description Jimmy Ohn 2015-02-13 11:14:12 UTC
In pulsesink_query function, We uses the 'switch' command accoding to GST_QUERY_TYPE. If type of query is the caps, we can't find the 'break' command.
It's just return the TRUE or FALSE value directly in this case.
So, I think that it should modify code for readability.
Comment 1 Jimmy Ohn 2015-02-13 11:19:14 UTC
Created attachment 296762 [details] [review]
pulsesink-Enhance-code-readability-in-pulsesink_query

This following patch enhance the code readability in pulsesink_query function.
Comment 2 Tim-Philipp Müller 2015-02-13 11:34:35 UTC
Comment on attachment 296762 [details] [review]
pulsesink-Enhance-code-readability-in-pulsesink_query

>+  gboolean ret = FALSE;
> 
>   switch (GST_QUERY_TYPE (query)) {
>     case GST_QUERY_CAPS:
>@@ -3188,9 +3188,8 @@ gst_pulsesink_query (GstBaseSink * sink, GstQuery * query)
>         gst_query_set_caps_result (query, caps);
>         gst_caps_unref (caps);
>         return TRUE;
>-      } else {
>-        return FALSE;
>       }
>+      break;
>     }

Thanks for the patch. I think it's okay, even if I don't think that there's a huge difference in readability here :)

You could go one step further then and turn the 'return TRUE' above into a ret=TRUE; (and then let it go on to the break). That would be more in line with other GStreamer code (avoiding returns in the middle of such functions).
Comment 3 Jimmy Ohn 2015-02-13 11:57:36 UTC
Created attachment 296769 [details] [review]
pulsesink-Enhance-code-readability-in-pulsesink_query

This following patch enhance the code readability in pulsesink_query function.
Comment 4 Jimmy Ohn 2015-02-13 12:01:01 UTC
Thank you for your comment.
I love gstreamer very much~!
Comment 5 Tim-Philipp Müller 2015-02-13 22:28:47 UTC
Thanks, pushed to master:

commit f9a8f0ebfea028e894b51de4215e27cebd784b09
Author: Jimmy Ohn <yongjin.ohn@lge.com>
Date:   Fri Feb 13 19:43:16 2015 +0900

    pulsesink: Enhance code readability in pulsesink_query
    
    In pulsesink_query function, we use a switch for the query
    type. In the CAPS case, there is no 'break', instead we
    return right away. Use a break and return at the end of
    the function instead for better code readability.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=744461