PSHTML icon indicating copy to clipboard operation
PSHTML copied to clipboard

Add function: New-PSHTMLChartDataSet

Open Stephanevg opened this issue 7 years ago • 5 comments

*** Edit ***

After a discussion, this is what will be implemented:

  • [X] A function called New-PSHTMLChartDataSet
  • [ ] The function must call the existing following functions:
    • New-PSHTMLChartBarDataSet
    • New-PSHTMLChartPieDataSet
    • New-PSHTMLChartLineDataSet
    • New-PSHTMLChartDoughnutDataSet
  • [ ] New-PSHTMLChartDataSet must have valid pester tests (Can re-use the ones from New-PSHTMLChartBarDataSet etc...)
  • [ ] New-PSHTMLChartDataSet must have Comment based help.
  • [ ] New-PSHTMLChartDataSet must have documentation in /docs/New-PSHTMLChartDataSet.md

First draft is located here --> https://github.com/Stephanevg/PSHTML/blob/Feature_Charts/PSHTML/Classes/Public/Chart.ps1

Function must be located in Chart.ps1 and cannot be moved to a seperate file.

Hi, I would like to discuss here the naming convention used for the Chart cmdlets.

We would have several types of cmdlets:

  1. The ones that generate the DataSet(s)
  2. Creation of the Chart

General

When it comes to charts in PSHTML, I would suggest we prefix all the cmdlets with Chart: EG: New-PSHTMLChart(Creates an actual chart) New-PSHTMLChartBarDataSetcreates aBar Chart dataset.

Like this, the user would see quickly, through intelisse, what is possible when it comes to charts.

DataSet

Currently, for complixity reasons, I have implemented a unique function per ChartType:

  • New-PSHTMLChartBarDataSet
  • New-PSHTMLChartPieDataSet
  • New-PSHTMLChartLineDataSet
  • New-PSHTMLChartDoughnutDataSet

According for which graph type the dataset will be used, properties and options will be different. To implement this quickly enough, I have designed a function per type, which has the correct properties available.

I would like to discuss the naming convention of these charts: For some reason, I think they would be better named, like this: New-PSHTMLChartDataSet<Type> EG: New-PSHTMLChartDataSetBar New-PSHTMLChartDataSetLine New-PSHTMLChartDataSetPie New-PSHTMLChartDataSetDoughnut

I would also like to discuss if it would make sense to add another function, that would create the right dataset. Something like this:

New-PSHTMLChartDataSet -Type Bar

The different type of dataset that would be created can be handled using ParameterSets. I am just afraid that this might end up into a pretty complex function, which would end up to be difficult to test and maintain.

What would be the pros and cons from each way in your opinion?

Chart creation

Currently, the cmdlet that allows to creates a chart is 'logically' named: New-PSHTMLChart It accepts a parameter -Type which accepts an Enum value of 'ChartType'. These contain all the current support charts (Pie,Line,Bar, Doughnut).

I don't think we need to change anything to this cmdlet, but I am open for discussions.

Stephanevg avatar Dec 10 '18 09:12 Stephanevg

I like the addition of -type in the function New-PSHTMLChartDataSet, I find it cleaner. Putting the type in the function does some very long function names. Especially since you have already done it for New-PSHTMLChart if I understand correctly ;-)

LaurentLienhard avatar Dec 10 '18 09:12 LaurentLienhard

You understood it correctly @LaurentLienhard I also think that it is more 'user friendly' with the parameter type. The big draw back, is that that function will end up to be HUGE. Perhaps we can keep these functions (just rename them perhaps?) and use them as private functions, which would be called from the main New-PSHTMLChartDataSet That would allow (at least in a first step) to delegate the complexity in child functions, and will avoid the main function to be too cluttered.

Stephanevg avatar Dec 10 '18 09:12 Stephanevg

Perhaps we can keep these functions (just rename them perhaps?) and use them as private functions, which would be called from the main New-PSHTMLChartDataSet

This is in my opinion the way to go here. Even tho it seems like a bit of a pain to maintain it from a developers perspective. I do believe that it's the nicest way to use it as an enduser.

Also the testability is way better if you have multiple little functions rather than one big one.

bateskevin avatar Dec 10 '18 10:12 bateskevin

Agreed, it will make testing a bit easier. (And potential bugs, would be 'isolated')

Stephanevg avatar Dec 10 '18 11:12 Stephanevg

Ok, I have written a first implementation here --> https://github.com/Stephanevg/PSHTML/blob/Feature_Charts/PSHTML/Classes/Public/Chart.ps1

I have added the label 'Up-For-Grabs' so someone can potentially jump in, and finish the work.

Stephanevg avatar Jan 06 '19 11:01 Stephanevg