pachabhaiya / nepali-calendar

Nepali calendar - Converts Gregorian calendar (A.D.) to Bikram Sambat (B.S.) and vice-versa.
MIT License
8 stars 7 forks source link

Some ideas on design #6

Open ojhaujjwal opened 9 years ago

ojhaujjwal commented 9 years ago

I am not happy with the design. So, I am proposing a completely new API.

<?php
namespace NepaliCalender;

class NepaliDate
{
    // these properties should be private/protected
    // kept public here just for demonstration purpose
    // getters should be used to access these
    public $year;
    public $month;
    public $day;
    // corresponding instance of \DateTime for the Nepali date
    public $dateTime;

    protected function __construct($year, $month, $day, \DateTime $dateTime)
    {
        $this->year = $year;
        $this->month = $month;
        $this->day = $day;
        $this->dateTime = $dateTime;
    }

    /**
     * Converts Nepali date to English date
     *
     * @param int $year     Nepali year
     * @param int $month    Nepali month
     * @param int $day      Nepali day
     * @return NepaliDate
     */ 
    public static function fromNepaliCalender($year, $month, $day)
    {
        // calculate English date
        $dateTime = \DateTime::createFromFormat("$englishYear-$englishMonth-$englishDay", 'Y-m-d', new \DateTimeZone('Asia/Kathmandu'));

        return new self($year, $month, $day, $dateTime);
    }

    /**
     * Converts English date to Nepali date
     *
     * @param DateTime $dateTime
     * @return NepaliDate
     */ 
    public static function fromGregorianCalender(\DateTime $dateTime)
    {
        $dateTime = clone $dateTime;
        $dateTime->setTimezone(new \DateTimeZone('Asia/Kathmandu'));

        // calculate Nepali date
        return new self($year, $month, $day, $dateTime);    
    }
}

?>

So, there will be one class which the user will be care. The two public methods will handle the conversion. But, this conversion part may be splitted to other classes.

How to use it

Suppose, user has entered nepali date in form, and we want to convert it to English date and get the instance of DateTime

<?php
$nepaliYear = $_POST['nepali_year'];
$nepaliMonth = $_POST['nepali_month'];
$nepaliDay = $_POST['nepali_day'];

// don't forget to filter and sanitize user input

$nepaliDate = NepaliCalender\NepaliDate::fromNepaliCalender($nepaliYear, $nepaliMonth, $nepaliDay, new DateTimeZone('UTC'));
$dateTime = $nepaliDate->dateTime;
?>

Suppose, we have english date and we want to convert to nepali date for displaying purpose:

<?php
$nepaliDate = NepaliCalender\NepaliDate::fromGregorianCalender($dateTime);
// tah dah
?>

ping @pachabhaiya @surajprogrammez @rajibmp

ghost commented 9 years ago

Its much improvement overall the previous design... personally I'd not allow public access to properties for return value, instead return it via methods (anyhow, personal preference)....

Great :+1:

ojhaujjwal commented 9 years ago

I have said in the comments that I added public properties for demonstration purposes only, private/protected properties and getters should be used!

pachabhaiya commented 9 years ago

:+1: Sounds good to me.

@surajprogrammez can you also give some feedbacks on this new concept by @ojhaujjwal

ghost commented 9 years ago

Just a quick test via Boris: After loading the class...

[2] boris> $y = 2014; 
// 2014
[3] boris> $m = 11;
// 11
[4] boris> $d = 29;
// 29
[5] boris> $da = NepaliDate::fromNepaliCalender($y, $m, $d, new DateTimeZone('UTC'));
PHP Catchable fatal error:  Argument 4 passed to NepaliDate::__construct() must be an instance of DateTime, boolean given, called in phar:///usr/local/bin/boris/lib/Boris/EvalWorker.php(133) : eval()'d code on line 39 and defined in phar:///usr/local/bin/boris/lib/Boris/EvalWorker.php(133) : eval()'d code on line 17
PHP Stack trace:
PHP   1. {main}() /usr/local/bin/boris:0
PHP   2. require() /usr/local/bin/boris:10
PHP   3. Boris\Boris->start() phar:///usr/local/bin/boris/bin/boris:15
PHP   4. Boris\EvalWorker->start() phar:///usr/local/bin/boris/lib/Boris/Boris.php:171
PHP   5. eval() phar:///usr/local/bin/boris/lib/Boris/EvalWorker.php:133
PHP   6. NepaliDate::fromNepaliCalender() phar:///usr/local/bin/boris/lib/Boris/EvalWorker.php(133) : eval()'d code:1
PHP   7. NepaliDate->__construct() phar:///usr/local/bin/boris/lib/Boris/EvalWorker.php(133) : eval()'d code:39

Not sure why passed Object is read as boolean type... I tried to create the object before passing...

[13] boris> $al = new DateTimeZone('UTC');
// object(DateTimeZone)(
//   'timezone_type' => 3,
//   'timezone' => 'UTC'
// )
[16] boris> $da = NepaliDate::fromNepaliCalender($y, $m, $d, $al);
PHP Catchable fatal error:  Argument 4 passed to NepaliDate::__construct() must be an instance of DateTime, boolean given, called in phar:///usr/local/bin/boris/lib/Boris/EvalWorker.php(133) : eval()'d code on line 39 and defined in phar:///usr/local/bin/boris/lib/Boris/EvalWorker.php(133) : eval()'d code on line 17
PHP Stack trace:
PHP   1. {main}() /usr/local/bin/boris:0
PHP   2. require() /usr/local/bin/boris:10
PHP   3. Boris\Boris->start() phar:///usr/local/bin/boris/bin/boris:15
PHP   4. Boris\EvalWorker->start() phar:///usr/local/bin/boris/lib/Boris/Boris.php:171
PHP   5. eval() phar:///usr/local/bin/boris/lib/Boris/EvalWorker.php:133
PHP   6. NepaliDate::fromNepaliCalender() phar:///usr/local/bin/boris/lib/Boris/EvalWorker.php(133) : eval()'d code:1
PHP   7. NepaliDate->__construct() phar:///usr/local/bin/boris/lib/Boris/EvalWorker.php(133) : eval()'d code:39

Funny enough, it is recognized as object outside the class

[17] boris> var_dump($al);
class DateTimeZone#8 (2) {
  public $timezone_type =>
  int(3)
  public $timezone =>
  string(3) "UTC"
}
// NULL
ghost commented 9 years ago

My above comment doesn't have any concrete value, as it was rough test...

but if we are creating a Wrapper class around the AdToBs and BsToAd, then I guess it can be achieved via small getter and setter, as you ( @ojhaujjwal ) mentioned in your previous issue, there might not be use of this class at all after all (if) we are just passing on the value, which can be done via those classes itself... my 2 cents.

ojhaujjwal commented 9 years ago

I have not tested it. It is just a simple POC on the new design!

ojhaujjwal commented 9 years ago

Also, I made changes to the fromNepaliCalender method.

ojhaujjwal commented 9 years ago

Please don't start implementing. It has some flaws as I detected. I will create a PR when I have time. I currently have exams for the 2nd semester. So, after these exams are finished and when I have time, I will create a PR. Till then, stay tuned!