New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formatting and null object error #462

Merged
merged 2 commits into from Jul 30, 2018

Conversation

Projects
None yet
2 participants
@jeremeguenther
Collaborator

jeremeguenther commented May 24, 2018

A couple of formatting change.
Also, in the new custom exception is a null object bug that has been fixed.

Formatting and null object error
A couple of formatting change.
Also, in the new custom exception is a null object bug that has been fixed.
@jeremeguenther

This comment has been minimized.

Collaborator

jeremeguenther commented Jul 10, 2018

Any chance we can get this pull request merged?

@@ -553,7 +553,7 @@ namespace <%=DALNameSpace%>
CommandText = command.CommandText;
CommandTimeout = command.CommandTimeout;
CommandType = command.CommandType.ToString();
ConnectionString = command.Connection.ConnectionString;
ConnectionString = command.Connection == null ? string.Empty : command.Connection.ConnectionString;

This comment has been minimized.

@niemyjski

niemyjski Jul 11, 2018

Contributor

Can we change this to command.Connection != null ? command.Connection.ConnectionString : null;

This comment has been minimized.

@jeremeguenther

jeremeguenther Jul 11, 2018

Collaborator

I will make this change. I have a number of additional changes to add to this commit as well.

@@ -732,10 +733,13 @@ WriteRelationshipPropertyString();
/// </returns>
public bool IsPropertyChanged(<%= GetClassName(SourceTable, ClassNameFormat.Column) %> column)
{
if (_originalData == null)

This comment has been minimized.

@niemyjski

niemyjski Jul 11, 2018

Contributor

Any reason for this change?

This comment has been minimized.

@jeremeguenther

jeremeguenther Jul 11, 2018

Collaborator

If property tracking has not been initialized, then _originalData is null when this method is called and blows up.
Another option would be to throw a custom error telling the user to initialize tracking.

@niemyjski

This comment has been minimized.

Contributor

niemyjski commented Jul 11, 2018

Yes, sorry for the delay. Can you please review my comments above and I'll get this merged asap.

Formatting, fixes, and features
- Bring some files formatting in line with the Microsoft Visual studio standard.
- fix some names
- Add single column table/view handling to the Utility file
- Add viewcolumns meta data to the EntityViewBase file
- Modify IsPropertyChanged to throw descriptive exception
- Modify NetTiers.cst to handle relative core directory paths.
@jeremeguenther

This comment has been minimized.

Collaborator

jeremeguenther commented Jul 11, 2018

I stacked another commit on top of this one. It includes the changes you asked for for the first commit as well as these items.

  • Bring some files formatting in line with the Microsoft Visual studio standard.
  • fix some names
  • Add single column table/view handling to the Utility file
  • Add viewcolumns meta data to the EntityViewBase file
  • Modify IsPropertyChanged to throw descriptive exception
  • Modify NetTiers.cst to handle relative core directory paths.
/// The name of the underlying database view's columns.
/// </summary>
[BrowsableAttribute(false), XmlIgnoreAttribute()]
public string[] ViewColumns

This comment has been minimized.

@niemyjski

niemyjski Jul 13, 2018

Contributor

I'm really not sure why we'd want to ever expose this.

This comment has been minimized.

@jeremeguenther

jeremeguenther Jul 13, 2018

Collaborator

It currently exposed for tables, just not views, this just brings views in line with how tables are generated.
Also, I use it in reflection when I need to know the original name of the database column before it goes through NetTiers normalization process. I think I would prefer some sort of dictionary mapping, and maybe some day i'll do that, but for now copying the way tables are handled was enough.

@niemyjski

This comment has been minimized.

Contributor

niemyjski commented Jul 13, 2018

Changes look good, I'm just not sure why'd we want to expose the view columns.

@niemyjski niemyjski merged commit cf0199b into netTiers:master Jul 30, 2018

@niemyjski

This comment has been minimized.

Contributor

niemyjski commented Jul 30, 2018

Thanks for the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment