Skip to main content
added 144 characters in body
Source Link
Flambino
  • 33.3k
  • 2
  • 46
  • 90

But actually, there's little point in having those static properties at all. TheFor instacne, the format method could be rewritten as

... and it wouldn't need to have access to any class properties at all (I'd probably call it build_query or build_request while I'm at it, since format isn't terribly descriptive).

SimilarlyBut of course, you can skip the format method entirely. It's private, and there's only one other method in the class, so there's only one place it'll be called. (If there were two or more places it could be called from, then a separate method would definitely make sense, of course.)

You also don't need to store anything fromgoing on in status() in static properties. $data = ... accomplishes the same as self::$data = ... (without having to type self::). Same for self::$result.

And of course, you can skip the format method entirely. It's private, and there's only one other method in the class, so there's only one place it'll be called.

But actually, there's little point in having those static properties at all. The format method could be rewritten as

and it wouldn't need to have access to any class properties at all (I'd probably call it build_query or build_request while I'm at it, since format isn't terribly descriptive).

Similarly, you don't need to store anything from status in static properties. $data = ... accomplishes the same as self::$data = ... (without having to type self::). Same for self::$result.

And of course, you can skip the format method entirely. It's private, and there's only one other method in the class, so there's only one place it'll be called.

But actually, there's little point in having those static properties at all. For instacne, the format method could be rewritten as

... and it wouldn't need to have access to any class properties at all (I'd probably call it build_query or build_request while I'm at it, since format isn't terribly descriptive).

But of course, you can skip the format method entirely. It's private, and there's only one other method in the class, so there's only one place it'll be called. (If there were two or more places it could be called from, then a separate method would definitely make sense, of course.)

You also don't need to store anything going on in status() in static properties. $data = ... accomplishes the same as self::$data = ... (without having to type self::). Same for self::$result.

added 13 characters in body
Source Link
Flambino
  • 33.3k
  • 2
  • 46
  • 90

Not sure why the properties are marked public, since they're overwritten each time Noti::status() is called. There's no real point in having external code access Noti::$user, for instance (external code can just call Config::get() if it wants the values). So protected or private would be better.

But actually, there's little point in having those static properties at all. The format method could be rewritten as

private static function format($title, $text) {
    return http_build_query(array(
        'app' => Config::get('noti.app_id'),
        'user' => Config::get('noti.user_token'),
        'notification[title]' => $title,
        'notification[text]' => $text
    ));
}

and it wouldn't need to have access to any class properties at all (I'd probably call it build_query or build_request while I'm at it, since format isn't terribly descriptive).

Similarly, you don't need to store anything from status in static properties. $data = ... accomplishes the same as self::$data = ... (without having to type self::). Same for self::$result.

And of course, you can skip the format method entirely. It's private, and there's only one other method in the class, so there's only one place it'll be called.

In the end, this should do the same thing

abstract class Noti {
    public static function status($title = null, $text = null) {
        if(empty($title) || empty($text)) return false;
        
        $data = http_build_query(array(
            'app'  => Config::get('noti.app_id'),
            'user' => Config::get('noti.user_token'),
            'notification[title]' => $title,
            'notification[text]'  => $text
        ));
        
        $url = Config::get('noti.url') . "add";
        
        $ch = curl_init($url);
        curl_setopt($ch, CURLOPT_POST, 1);
        curl_setopt($ch, CURLOPT_POSTFIELDS, $data);
        $result = curl_exec($ch);
        curl_close($ch);

        return $result;
    }
}

Not sure why the properties are marked public, since they're overwritten each time Noti::status() is called. There's no real point in having external code access Noti::$user, for instance. So protected or private would be better.

But actually, there's little point in having those static properties at all. The format method could be rewritten as

private static function format($title, $text) {
    return http_build_query(array(
        'app' => Config::get('noti.app_id'),
        'user' => Config::get('noti.user_token'),
        'notification[title]' => $title,
        'notification[text]' => $text
    ));
}

and it wouldn't need to have access to any class properties at all (I'd probably call it build_query or build_request while I'm at it, since format isn't terribly descriptive).

Similarly, you don't need to store anything from status in static properties. $data = ... accomplishes the same as self::$data = ... (without having to type self::). Same for self::$result.

And of course, you can skip the format method entirely. It's private, and there's only one other method, so there's only one place it'll be called.

In the end, this should do the same thing

abstract class Noti {
    public static function status($title = null, $text = null) {
        if(empty($title) || empty($text)) return false;
        
        $data = http_build_query(array(
            'app'  => Config::get('noti.app_id'),
            'user' => Config::get('noti.user_token'),
            'notification[title]' => $title,
            'notification[text]'  => $text
        ));
        
        $url = Config::get('noti.url') . "add";
        
        $ch = curl_init($url);
        curl_setopt($ch, CURLOPT_POST, 1);
        curl_setopt($ch, CURLOPT_POSTFIELDS, $data);
        $result = curl_exec($ch);
        curl_close($ch);

        return $result;
    }
}

Not sure why the properties are marked public, since they're overwritten each time Noti::status() is called. There's no real point in having external code access Noti::$user, for instance (external code can just call Config::get() if it wants the values). So protected or private would be better.

But actually, there's little point in having those static properties at all. The format method could be rewritten as

private static function format($title, $text) {
    return http_build_query(array(
        'app' => Config::get('noti.app_id'),
        'user' => Config::get('noti.user_token'),
        'notification[title]' => $title,
        'notification[text]' => $text
    ));
}

and it wouldn't need to have access to any class properties at all (I'd probably call it build_query or build_request while I'm at it, since format isn't terribly descriptive).

Similarly, you don't need to store anything from status in static properties. $data = ... accomplishes the same as self::$data = ... (without having to type self::). Same for self::$result.

And of course, you can skip the format method entirely. It's private, and there's only one other method in the class, so there's only one place it'll be called.

In the end, this should do the same thing

abstract class Noti {
    public static function status($title = null, $text = null) {
        if(empty($title) || empty($text)) return false;
        
        $data = http_build_query(array(
            'app'  => Config::get('noti.app_id'),
            'user' => Config::get('noti.user_token'),
            'notification[title]' => $title,
            'notification[text]'  => $text
        ));
        
        $url = Config::get('noti.url') . "add";
        
        $ch = curl_init($url);
        curl_setopt($ch, CURLOPT_POST, 1);
        curl_setopt($ch, CURLOPT_POSTFIELDS, $data);
        $result = curl_exec($ch);
        curl_close($ch);

        return $result;
    }
}
Source Link
Flambino
  • 33.3k
  • 2
  • 46
  • 90

Not sure why the properties are marked public, since they're overwritten each time Noti::status() is called. There's no real point in having external code access Noti::$user, for instance. So protected or private would be better.

But actually, there's little point in having those static properties at all. The format method could be rewritten as

private static function format($title, $text) {
    return http_build_query(array(
        'app' => Config::get('noti.app_id'),
        'user' => Config::get('noti.user_token'),
        'notification[title]' => $title,
        'notification[text]' => $text
    ));
}

and it wouldn't need to have access to any class properties at all (I'd probably call it build_query or build_request while I'm at it, since format isn't terribly descriptive).

Similarly, you don't need to store anything from status in static properties. $data = ... accomplishes the same as self::$data = ... (without having to type self::). Same for self::$result.

And of course, you can skip the format method entirely. It's private, and there's only one other method, so there's only one place it'll be called.

In the end, this should do the same thing

abstract class Noti {
    public static function status($title = null, $text = null) {
        if(empty($title) || empty($text)) return false;
        
        $data = http_build_query(array(
            'app'  => Config::get('noti.app_id'),
            'user' => Config::get('noti.user_token'),
            'notification[title]' => $title,
            'notification[text]'  => $text
        ));
        
        $url = Config::get('noti.url') . "add";
        
        $ch = curl_init($url);
        curl_setopt($ch, CURLOPT_POST, 1);
        curl_setopt($ch, CURLOPT_POSTFIELDS, $data);
        $result = curl_exec($ch);
        curl_close($ch);

        return $result;
    }
}