getndazn / kopytko-utils

A collection of modern utility functions for Brightscript applications
MIT License
6 stars 4 forks source link

feat: added deep print function which allows to print objects for specified nested levels #11

Closed kuldeepk-dazn closed 1 year ago

kuldeepk-dazn commented 2 years ago

What did you implement:

Closes #ROKU-1310

Deep print function for the objects which prints nested objects till the specified nesting levels

How did you implement it:

It is a recursive function which calles itself until the specified nesting levels and print the object properties for each level.

How can we verify it:

Screenshot 2022-06-13 at 12 20 00 Screenshot 2022-06-13 at 12 17 37

Todos:

Is this ready for review?: YES Is it a breaking change?: NO

RadoslawZambrowski commented 2 years ago

Yeah, I agree @pawelhertman. I also have some doubts about putting such code in ObjectUtils. Eventually, maybe something like PrintUtils with printAA(obj, nestedLevel) could be extracted if you think it's worth keeping it in kopytko-utils?

bchelkowski commented 2 years ago

I would just keep it as a separate, standalone function, as it is just easier to use.

pawelhertman commented 2 years ago

@bchelkowski but does it makes sense to put it in Kopytko Utils? If it was a part of a bigger logging/debugging tool, that would be great, but now it looks like a single function from a totally different world.

bchelkowski commented 2 years ago

And where to put it? We won't create another package just for one debugging function. In the future, it may be moved to the koptyko/debugging-tools, if this will happen.

RadoslawZambrowski commented 2 years ago

@pawelhertman @bchelkowski ok, so I think we would need to decide if we keep it in kopytko-utils (outside ObjectUtils, as separate function or in another util object e.g. Print/DebugUtils) or we don't add it to kopytko-utils at all and such code should be added in the client app code. Do I get it right?

tomek-r commented 2 years ago

It seems such code should be part of bigger DebugUtils/Logger entity. If left in the production code alone it would affect performance because of the heavy looping and nesting, so I would expect such entity can be enabled/disabled via public interface. What do you think?

bchelkowski commented 2 years ago

So I would keep it rather in one function, or do you see any other method that could be added to the Logger/DebugUtils/Print entity? If we will have more, we could create another npm package for it, and get rid of the problem of having this code in the production (assuming a project has a plugin to remove it, or we will implement tree-shaking for the packager). I wouldn't add any additional code to enable/disable it as this depends from the developer if he uses it in the code. It is exactly the same if he left console.log in the production code. If you won't use this function in the production code, there won't be any performance issues, we can emphasize this in the documentation.

bchelkowski commented 1 year ago

For now, I'm closing the PR. If we will still feel that we need this we can reopen it. However, using this function with the necessity for importing it each time would lead to not using it at all in the end.